diff --git a/cmd/ragflow_server.go b/cmd/ragflow_server.go index ddd326b5b7..c100e91b89 100644 --- a/cmd/ragflow_server.go +++ b/cmd/ragflow_server.go @@ -38,6 +38,7 @@ import ( "ragflow/internal/storage" "ragflow/internal/syncer" "ragflow/internal/tokenizer" + "runtime" "strconv" "strings" "syscall" @@ -494,7 +495,7 @@ func runIngestor(args *serverArgs) error { } defer tokenizer.Close() - ingestor := ingestion.NewIngestor(*args.name, 2, []string{"pdf", "docx", "txt"}) + ingestor := ingestion.NewIngestor(*args.name, int32(runtime.NumCPU()), []string{"pdf", "docx", "txt"}) go func() { err := ingestor.Start() @@ -803,6 +804,7 @@ func startServer(config *server.Config) { ) componentsSvc := service.NewComponentsService() componentsHandler := handler.NewComponentsHandler(componentsSvc) + pipelineHandler := handler.NewPipelineHandler() // Initialize router r := router.NewRouter(authHandler, @@ -833,7 +835,8 @@ func startServer(config *server.Config) { fileCommitHandler, openaiChatHandler, botHandler, - componentsHandler) + componentsHandler, + pipelineHandler) // Create Gin enginegit diff diff --git a/internal/common/parser_config.go b/internal/common/parser_config.go index 08ee98d7ee..50cbb24ddd 100644 --- a/internal/common/parser_config.go +++ b/internal/common/parser_config.go @@ -53,7 +53,7 @@ func DeepMergeMaps(base, override map[string]interface{}) map[string]interface{} // GetParserConfig builds the final parser_config stored on a dataset: // base defaults -> chunk-method defaults -> caller overrides. -func GetParserConfig(chunkMethod string, parserConfig map[string]interface{}) map[string]interface{} { +func GetParserConfig(parserID string, parserConfig map[string]interface{}) map[string]interface{} { baseDefaults := map[string]interface{}{ "table_context_size": 0, "image_context_size": 0, @@ -107,15 +107,8 @@ func GetParserConfig(chunkMethod string, parserConfig map[string]interface{}) ma "raptor": map[string]interface{}{"use_raptor": false}, "graphrag": map[string]interface{}{"use_graphrag": false}, }, - "knowledge_graph": { - "chunk_token_num": 8192, - "delimiter": "\\n", - "entity_types": []interface{}{"organization", "person", "location", "event", "time"}, - "raptor": map[string]interface{}{"use_raptor": false}, - "graphrag": map[string]interface{}{"use_graphrag": false}, - }, } - merged := DeepMergeMaps(baseDefaults, defaultConfigs[chunkMethod]) + merged := DeepMergeMaps(baseDefaults, defaultConfigs[parserID]) return DeepMergeMaps(merged, parserConfig) } diff --git a/internal/handler/pipeline.go b/internal/handler/pipeline.go new file mode 100644 index 0000000000..925d615e1c --- /dev/null +++ b/internal/handler/pipeline.go @@ -0,0 +1,72 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package handler + +import ( + "github.com/gin-gonic/gin" + "go.uber.org/zap" + + "ragflow/internal/common" + pipelinepkg "ragflow/internal/ingestion/pipeline" +) + +// builtinPipelineLister is the registry surface PipelineHandler needs. +// Defined as an interface so handler tests can inject a fake without +// touching the embedded template FS. +type builtinPipelineLister interface { + List() []*pipelinepkg.BuiltinPipelineMeta +} + +// PipelineHandler handles pipeline endpoints. +// Pipelines are the top-level resource for ingestion pipeline templates. +// Built-in templates are shipped with the binary; user-defined templates +// (canvas DSL) may be added later. +type PipelineHandler struct { + registry builtinPipelineLister +} + +// NewPipelineHandler builds a PipelineHandler backed by the embedded +// default registry. If the registry fails to load (malformed template), +// the handler is returned with a nil registry and ListPipelines serves +// an empty list rather than crashing the route. +func NewPipelineHandler() *PipelineHandler { + registry, err := pipelinepkg.DefaultRegistry() + if err != nil { + common.Warn("failed to load builtin pipeline registry", zap.Error(err)) + return &PipelineHandler{} + } + return &PipelineHandler{registry: registry} +} + +// ListPipelines GET /api/v1/pipelines +// Returns available pipeline templates. When type=builtin (or when no +// type is specified), returns the built-in pipeline catalog shipped with +// the binary. Support for user-defined pipelines may be added later. +// This is public static data, so no auth is required. +func (h *PipelineHandler) ListPipelines(c *gin.Context) { + if h == nil || h.registry == nil { + common.SuccessWithData(c, []*pipelinepkg.BuiltinPipelineMeta{}, "success") + return + } + + pipelineType := c.Query("type") + // For now only builtin pipelines exist; user-defined may be added later. + // When type is empty or "builtin", return the builtin catalog. + _ = pipelineType + + common.SuccessWithData(c, h.registry.List(), "success") +} diff --git a/internal/handler/pipeline_test.go b/internal/handler/pipeline_test.go new file mode 100644 index 0000000000..c95ec13cbf --- /dev/null +++ b/internal/handler/pipeline_test.go @@ -0,0 +1,158 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package handler + +import ( + "encoding/json" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + + "ragflow/internal/common" + pipelinepkg "ragflow/internal/ingestion/pipeline" +) + +// fakePipelineLister is a test double for builtinPipelineLister. +type fakePipelineLister struct { + items []*pipelinepkg.BuiltinPipelineMeta +} + +func (f fakePipelineLister) List() []*pipelinepkg.BuiltinPipelineMeta { return f.items } + +func pipelineCtx() (*gin.Context, *httptest.ResponseRecorder) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/api/v1/pipelines?type=builtin", nil) + return c, w +} + +func TestPipelineHandler_ListPipelines_TypeBuiltin(t *testing.T) { + items := []*pipelinepkg.BuiltinPipelineMeta{ + { + ParserID: "general", + Title: "General", + Description: "general desc", + Filename: "ingestion_pipeline_general.json", + DSL: map[string]any{"components": map[string]any{}}, + }, + { + ParserID: "book", + Title: "Book", + Filename: "ingestion_pipeline_book.json", + DSL: map[string]any{"components": map[string]any{}}, + }, + } + h := &PipelineHandler{registry: fakePipelineLister{items: items}} + + c, w := pipelineCtx() + h.ListPipelines(c) + + if w.Code != 200 { + t.Fatalf("status = %d, want 200", w.Code) + } + var resp struct { + Code int `json:"code"` + Data []struct { + ParserID string `json:"parser_id"` + Title string `json:"title"` + DSL map[string]any `json:"dsl"` + } `json:"data"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if resp.Code != int(common.CodeSuccess) { + t.Fatalf("code = %d, want %d", resp.Code, int(common.CodeSuccess)) + } + if len(resp.Data) != 2 { + t.Fatalf("len(data) = %d, want 2", len(resp.Data)) + } + if resp.Data[0].ParserID != "general" || resp.Data[0].Title != "General" { + t.Errorf("data[0] = %+v", resp.Data[0]) + } +} + +func TestPipelineHandler_ListPipelines_NoTypeParam(t *testing.T) { + items := []*pipelinepkg.BuiltinPipelineMeta{ + {ParserID: "general", Title: "General"}, + } + h := &PipelineHandler{registry: fakePipelineLister{items: items}} + + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/api/v1/pipelines", nil) + + h.ListPipelines(c) + + if w.Code != 200 { + t.Fatalf("status = %d, want 200", w.Code) + } + var resp struct { + Data []struct { + ParserID string `json:"parser_id"` + } `json:"data"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(resp.Data) != 1 { + t.Fatalf("len(data) = %d, want 1 (no type param defaults to builtin)", len(resp.Data)) + } +} + +// TestPipelineHandler_ListPipelines_RealRegistry verifies the production +// DefaultRegistry wires up correctly: the handler returns the actual +// embedded builtin templates (general, book, ...) and the legacy alias +// naive is hidden from the listing. +func TestPipelineHandler_ListPipelines_RealRegistry(t *testing.T) { + h := NewPipelineHandler() + if h == nil || h.registry == nil { + t.Fatal("NewPipelineHandler did not wire a registry") + } + + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/api/v1/pipelines?type=builtin", nil) + + h.ListPipelines(c) + + var resp struct { + Data []struct { + ParserID string `json:"parser_id"` + } `json:"data"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(resp.Data) == 0 { + t.Fatal("expected non-empty builtin pipeline list from real registry") + } + seen := map[string]bool{} + for _, item := range resp.Data { + seen[item.ParserID] = true + } + if !seen["general"] { + t.Error("real registry listing missing general") + } + if seen["naive"] { + t.Error("alias naive must be hidden from listing") + } +} diff --git a/internal/ingestion/component/tokenizer.go b/internal/ingestion/component/tokenizer.go index f86d87910f..61959fb26d 100644 --- a/internal/ingestion/component/tokenizer.go +++ b/internal/ingestion/component/tokenizer.go @@ -105,6 +105,7 @@ import ( "ragflow/internal/ingestion/component/globals" "ragflow/internal/ingestion/component/schema" "ragflow/internal/tokenizer" + "ragflow/internal/utility" ) const ComponentNameTokenizer = "Tokenizer" @@ -648,7 +649,7 @@ func tokenizeChunks(chunks []schema.ChunkDoc, titleStem string) error { } } if kw := ck.Keywords; kw != "" { - if err := ck.SetExtraValue("important_kwd", strings.Split(kw, ",")); err != nil { + if err := ck.SetExtraValue("important_kwd", utility.SplitKeywords(kw)); err != nil { return fmt.Errorf("Tokenizer: keyword list marshal: %w", err) } it, err := tokenizer.Tokenize(kw) diff --git a/internal/ingestion/component/tokenizer_test.go b/internal/ingestion/component/tokenizer_test.go index 486cf2b4a8..ccd14a17ce 100644 --- a/internal/ingestion/component/tokenizer_test.go +++ b/internal/ingestion/component/tokenizer_test.go @@ -27,11 +27,9 @@ import ( "math" "reflect" "strings" - "sync/atomic" "testing" "time" - "ragflow/internal/agent/runtime" "ragflow/internal/ingestion/component/schema" "ragflow/internal/tokenizer" ) @@ -49,102 +47,6 @@ func requireTokenizerPool(t *testing.T) { } } -// stubEmbedder records every call and returns canned vectors. -// Matches the Embedder contract: len(results) == len(texts). -type stubEmbedder struct { - calls atomic.Int32 - dim int - maxTokens int - delay time.Duration - err error - callInputs [][]string - resultsByCall []embeddingCallResult - callTokens []int -} - -type embeddingCallResult struct { - vectors [][]float64 - tokenCount int -} - -func (s *stubEmbedder) MaxTokens() int { - return s.maxTokens -} - -func (s *stubEmbedder) Encode(texts []string) ([]EmbeddingResult, error) { - s.calls.Add(1) - copied := append([]string(nil), texts...) - s.callInputs = append(s.callInputs, copied) - if s.delay > 0 { - time.Sleep(s.delay) - } - if s.err != nil { - return nil, s.err - } - callIdx := int(s.calls.Load()) - 1 - var cfg embeddingCallResult - if callIdx < len(s.resultsByCall) { - cfg = s.resultsByCall[callIdx] - } - out := make([]EmbeddingResult, len(texts)) - for i := range texts { - var v []float64 - if i < len(cfg.vectors) { - v = append([]float64(nil), cfg.vectors[i]...) - } else { - v = make([]float64, s.dim) - v[0] = float64(i + 1) - } - tokenCount := len(texts[i]) - if callIdx < len(s.callTokens) { - tokenCount = s.callTokens[callIdx] - } else if cfg.tokenCount > 0 { - tokenCount = cfg.tokenCount - } - out[i] = EmbeddingResult{Vector: v, TokenCount: tokenCount} - } - return out, nil -} - -// newStubEmbedder returns a stub embedder for instance-level resolver injection. -func newStubEmbedder(dim int) *stubEmbedder { - return &stubEmbedder{dim: dim} -} - -// withStubEmbedder constructs a TokenizerComponent with an instance-scoped -// resolver backed by a stub embedder. -func withStubEmbedder(t *testing.T, dim int) (*TokenizerComponent, *stubEmbedder) { - t.Helper() - stub := newStubEmbedder(dim) - comp, err := NewTokenizerComponentWithResolver(nil, func(_, _, _ string) (Embedder, error) { return stub, nil }) - if err != nil { - t.Fatalf("NewTokenizerComponentWithResolver: %v", err) - } - return comp.(*TokenizerComponent), stub -} - -// TestTokenizerComponent_Registered verifies init() enrollment -// under runtime.CategoryIngestion (Phase 4 / API endpoint depends -// on this contract). -func TestTokenizerComponent_Registered(t *testing.T) { - factory, cat, md, ok := runtime.DefaultRegistry.Lookup("Tokenizer") - if !ok { - t.Fatal("Tokenizer not registered in runtime.DefaultRegistry") - } - if cat != runtime.CategoryIngestion { - t.Errorf("category = %q, want %q", cat, runtime.CategoryIngestion) - } - if factory == nil { - t.Error("factory is nil") - } - if len(md.Inputs) == 0 { - t.Error("metadata.Inputs empty") - } - if len(md.Outputs) == 0 { - t.Error("metadata.Outputs empty") - } -} - // TestTokenizerComponent_Invoke_HappyPath drives three chunks // through both full_text tokenization and embedding. Verifies that // every chunk gains `content_ltks`, `content_sm_ltks`, and a @@ -214,56 +116,6 @@ func TestTokenizerComponent_Invoke_HappyPath(t *testing.T) { } } -// TestTokenizerComponent_Invoke_EmptyChunks covers the no-op branch: -// empty chunk list → empty output, no panic, no encoder call. -func TestTokenizerComponent_Invoke_EmptyChunks(t *testing.T) { - c, stub := withStubEmbedder(t, 4) - _ = stub - var err error - if err != nil { - t.Fatalf("NewTokenizerComponent: %v", err) - } - - out, err := c.Invoke(context.Background(), map[string]any{ - "output_format": "chunks", - "chunks": []map[string]any{}, - }) - if err != nil { - t.Fatalf("Invoke: %v", err) - } - chunks, _ := out["chunks"].([]map[string]any) - if len(chunks) != 0 { - t.Errorf("chunks len = %d, want 0", len(chunks)) - } - if stub.calls.Load() != 0 { - t.Errorf("embedder called %d times on empty input, want 0", stub.calls.Load()) - } - if got := out["embedding_token_consumption"]; got != 0 { - t.Errorf("embedding_token_consumption = %v, want 0", got) - } - if out["output_format"] != "chunks" { - t.Errorf("output_format = %v, want chunks", out["output_format"]) - } -} - -// TestTokenizerComponent_Invoke_NilChunks covers the nil-input -// branch: nil chunks list is treated as zero-length (matches -// python `kwargs.get("chunks")` with None). -func TestTokenizerComponent_Invoke_NilChunks(t *testing.T) { - c, stub := withStubEmbedder(t, 4) - _ = stub - out, err := c.Invoke(context.Background(), map[string]any{ - "output_format": "chunks", - }) - if err != nil { - t.Fatalf("Invoke: %v", err) - } - chunks, _ := out["chunks"].([]map[string]any) - if len(chunks) != 0 { - t.Errorf("chunks len = %d, want 0", len(chunks)) - } -} - // TestTokenizerComponent_Invoke_Unicode asserts CJK input // produces finite, non-negative token counts (plan §8 Q2: Go // `NumTokensFromString` over-counts CJK on tiktoken-init failure; @@ -408,35 +260,36 @@ func TestTokenizerComponent_Invoke_FullTextOnly(t *testing.T) { } } -func TestTokenizerComponent_Invoke_EmbeddingOnly(t *testing.T) { - cIntf, err := NewTokenizerComponentWithResolver(map[string]any{ - "search_method": []any{"embedding"}, - }, func(_, _, _ string) (Embedder, error) { - return newStubEmbedder(4), nil +// TestTokenizerComponent_Invoke_KeywordSplitCJK verifies important_kwd is +// split by the full ASCII+CJK delimiter set, not just ASCII comma. A Chinese +// LLM commonly emits CJK commas/semicolons even when asked for +// "comma-separated"; ASCII-only splitting would leave keywords glued together. +func TestTokenizerComponent_Invoke_KeywordSplitCJK(t *testing.T) { + requireTokenizerPool(t) + _, stub := withStubEmbedder(t, 4) + c, _ := NewTokenizerComponent(map[string]any{ + "search_method": []any{"full_text"}, }) - if err != nil { - t.Fatalf("NewTokenizerComponentWithResolver: %v", err) - } - out, err := cIntf.(*TokenizerComponent).Invoke(context.Background(), map[string]any{ - "name": "doc.pdf", + out, err := c.Invoke(context.Background(), map[string]any{ "output_format": "chunks", - "chunks": []map[string]any{{"text": "alpha bravo"}}, + "chunks": []map[string]any{{"text": "alpha", "keywords": "kw1，kw2；kw3"}}, }) if err != nil { t.Fatalf("Invoke: %v", err) } + if stub.calls.Load() != 0 { + t.Errorf("embedder should not be called in full_text-only mode, got %d", stub.calls.Load()) + } got, _ := out["chunks"].([]map[string]any) if len(got) != 1 { t.Fatalf("chunks len = %d, want 1", len(got)) } - if got[0]["q_4_vec"] == nil { - t.Fatalf("q_4_vec missing: %v", got[0]) + kwd, ok := got[0]["important_kwd"].([]string) + if !ok { + t.Fatalf("important_kwd should be []string, got %T", got[0]["important_kwd"]) } - if got[0]["content_ltks"] != nil || got[0]["content_sm_ltks"] != nil { - t.Fatalf("embedding-only mode should not emit full-text tokens: %v", got[0]) - } - if out["embedding_token_consumption"] == nil { - t.Fatal("embedding_token_consumption missing") + if len(kwd) != 3 { + t.Errorf("important_kwd must split CJK delimiters into 3 elements, got %d: %v", len(kwd), kwd) } } @@ -567,60 +420,6 @@ func TestTokenizerComponent_Invoke_HonorsTimeout(t *testing.T) { } } -// TestTokenizerComponent_InputsOutputs_NonEmpty verifies Phase 4 -// API metadata shape. -func TestTokenizerComponent_InputsOutputs_NonEmpty(t *testing.T) { - c, _ := NewTokenizerComponent(map[string]any{}) - ins := c.(*TokenizerComponent).Inputs() - outs := c.(*TokenizerComponent).Outputs() - if len(ins) == 0 { - t.Error("Inputs() empty") - } - if len(outs) == 0 { - t.Error("Outputs() empty") - } - for _, key := range []string{"chunks", "output_format"} { - if _, ok := outs[key]; !ok { - t.Errorf("Outputs() missing %q", key) - } - } - for _, key := range []string{"chunks", "name"} { - if _, ok := ins[key]; !ok { - t.Errorf("Inputs() missing %q", key) - } - } -} - -// TestTokenizerComponent_NewTokenizerComponent_Defaults verifies -// the Python default param values propagate. -func TestTokenizerComponent_NewTokenizerComponent_Defaults(t *testing.T) { - c, err := NewTokenizerComponent(nil) - if err != nil { - t.Fatalf("NewTokenizerComponent(nil): %v", err) - } - tc := c.(*TokenizerComponent) - if tc.param.FilenameEmbdWeight != 0.1 { - t.Errorf("filename_embd_weight = %v, want 0.1", tc.param.FilenameEmbdWeight) - } - if len(tc.param.Fields) != 1 || tc.param.Fields[0] != "text" { - t.Errorf("fields = %v, want [text]", tc.param.Fields) - } - if len(tc.param.SearchMethod) != 2 { - t.Errorf("search_method len = %d, want 2", len(tc.param.SearchMethod)) - } -} - -// TestTokenizerComponent_NewTokenizerComponent_BadParam covers -// the param-validation branch (invalid search_method value). -func TestTokenizerComponent_NewTokenizerComponent_BadParam(t *testing.T) { - _, err := NewTokenizerComponent(map[string]any{ - "search_method": []any{"unknown"}, - }) - if err == nil { - t.Fatal("expected param validation error, got nil") - } -} - // TestTokenizerComponent_Smoke_EndToEnd is the BLOCKER smoke test // (plan §8 R3). Drives 1 chunk of ~1000 tokens through the real // tokenizer and a stub embedder with no artificial latency, then @@ -697,6 +496,7 @@ func TestTokenizerComponent_Smoke_EndToEnd(t *testing.T) { } func TestTokenizerComponent_Embedding_MergesTitleAndContentVectors(t *testing.T) { + requireTokenizerPool(t) c, stub := withStubEmbedder(t, 2) stub.resultsByCall = []embeddingCallResult{ {vectors: [][]float64{{10, 20}}, tokenCount: 7}, @@ -722,6 +522,7 @@ func TestTokenizerComponent_Embedding_MergesTitleAndContentVectors(t *testing.T) } func TestTokenizerComponent_Embedding_UsesFilenameWeight(t *testing.T) { + requireTokenizerPool(t) cIntf, err := NewTokenizerComponentWithResolver(map[string]any{ "filename_embd_weight": 0.25, }, func(_, _, _ string) (Embedder, error) { @@ -752,6 +553,7 @@ func TestTokenizerComponent_Embedding_UsesFilenameWeight(t *testing.T) { } func TestTokenizerComponent_Embedding_EmptyNameWarnsAndUsesContentVector(t *testing.T) { + requireTokenizerPool(t) c, stub := withStubEmbedder(t, 2) stub.resultsByCall = []embeddingCallResult{{vectors: [][]float64{{2, 4}}, tokenCount: 5}} @@ -790,6 +592,7 @@ func TestTokenizerComponent_Embedding_EmptyNameWarnsAndUsesContentVector(t *test } func TestTokenizerComponent_Embedding_TruncatesByMaxTokensMinus10(t *testing.T) { + requireTokenizerPool(t) c, stub := withStubEmbedder(t, 2) stub.maxTokens = 12 longText := strings.Repeat("hello world ", 20) @@ -812,6 +615,7 @@ func TestTokenizerComponent_Embedding_TruncatesByMaxTokensMinus10(t *testing.T) } func TestTokenizerComponent_Embedding_SkipsEmptyCleanedTextsButReturnsZeroWhenAllSkipped(t *testing.T) { + requireTokenizerPool(t) c, stub := withStubEmbedder(t, 2) out, err := c.Invoke(context.Background(), map[string]any{ "name": "doc.pdf", @@ -839,6 +643,7 @@ func TestTokenizerComponent_Embedding_SkipsEmptyCleanedTextsButReturnsZeroWhenAl } func TestTokenizerComponent_Embedding_SetsTokenConsumptionIncludingTitleCall(t *testing.T) { + requireTokenizerPool(t) c, stub := withStubEmbedder(t, 2) stub.callTokens = []int{3, 5, 7} prevBatchSize := tokenizerEmbeddingBatchSize @@ -859,6 +664,7 @@ func TestTokenizerComponent_Embedding_SetsTokenConsumptionIncludingTitleCall(t * } func TestTokenizerComponent_Embedding_BatchesByConfiguredBatchSize(t *testing.T) { + requireTokenizerPool(t) c, stub := withStubEmbedder(t, 2) prevBatchSize := tokenizerEmbeddingBatchSize tokenizerEmbeddingBatchSize = 2 @@ -886,24 +692,6 @@ func TestTokenizerComponent_Embedding_BatchesByConfiguredBatchSize(t *testing.T) } } -func TestTokenizerComponent_Embedding_ZeroChunksStillEmitsConsumptionZero(t *testing.T) { - c, stub := withStubEmbedder(t, 2) - out, err := c.Invoke(context.Background(), map[string]any{ - "name": "doc.pdf", - "output_format": "chunks", - "chunks": []map[string]any{}, - }) - if err != nil { - t.Fatalf("Invoke: %v", err) - } - if got := stub.calls.Load(); got != 0 { - t.Fatalf("embedder calls = %d, want 0", got) - } - if got := out["embedding_token_consumption"]; got != 0 { - t.Fatalf("embedding_token_consumption = %v, want 0", got) - } -} - func floatSliceClose(got, want []float64) bool { if len(got) != len(want) { return false @@ -916,28 +704,6 @@ func floatSliceClose(got, want []float64) bool { return true } -func TestValidateTokenizerOutputs_FullTextMissingReturnsError(t *testing.T) { - err := validateTokenizerOutputs([]schema.ChunkDoc{{Text: "alpha"}}, []string{"full_text"}, []string{"text"}) - if err == nil || !strings.Contains(err.Error(), "missing full_text tokens") { - t.Fatalf("err = %v, want missing full_text tokens", err) - } -} - -func TestValidateTokenizerOutputs_EmbeddingMissingReturnsError(t *testing.T) { - err := validateTokenizerOutputs([]schema.ChunkDoc{{Text: "alpha"}}, []string{"embedding"}, []string{"text"}) - if err == nil || !strings.Contains(err.Error(), "missing embedding vector") { - t.Fatalf("err = %v, want missing embedding vector", err) - } -} - -func TestValidateTokenizerOutputs_BothModesFailWhenOneMissing(t *testing.T) { - ck := schema.ChunkDoc{Text: "alpha", ContentLtks: "tok", ContentSmLtks: "sm"} - err := validateTokenizerOutputs([]schema.ChunkDoc{ck}, []string{"full_text", "embedding"}, []string{"text"}) - if err == nil || !strings.Contains(err.Error(), "missing embedding vector") { - t.Fatalf("err = %v, want missing embedding vector", err) - } -} - func TestTokenizerComponent_InstanceResolversDoNotLeakAcrossComponents(t *testing.T) { requireTokenizerPool(t) compAIntf, err := NewTokenizerComponentWithResolver(nil, func(_, _, _ string) (Embedder, error) { @@ -974,22 +740,6 @@ func TestTokenizerComponent_InstanceResolversDoNotLeakAcrossComponents(t *testin } } -func TestValidateTokenizerOutputs_SymbolOnlyContentLtksIsEmptyFails(t *testing.T) { - // Simulates a chunk whose Text is a symbol/punctuation character that - // the C++ RAGAnalyzer tokenizer cannot produce tokens for (e.g. "·", ")", "("). - // After tokenizeChunks runs, ContentLtks and ContentSmLtks remain empty, - // and validateTokenizerOutputs must detect this as a failure. - ck := schema.ChunkDoc{ - Text: ")", - ContentLtks: "", - ContentSmLtks: "", - } - err := validateTokenizerOutputs([]schema.ChunkDoc{ck}, []string{"full_text"}, []string{"text"}) - if err == nil || !strings.Contains(err.Error(), "missing full_text tokens") { - t.Fatalf("err = %v, want missing full_text tokens", err) - } -} - func TestTokenizeChunks_SymbolOnlyTextFallsBackToRawText(t *testing.T) { requireTokenizerPool(t) chunks := []schema.ChunkDoc{ diff --git a/internal/ingestion/component/tokenizer_unit_test.go b/internal/ingestion/component/tokenizer_unit_test.go new file mode 100644 index 0000000000..d106979896 --- /dev/null +++ b/internal/ingestion/component/tokenizer_unit_test.go @@ -0,0 +1,324 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// Unit tests for the Tokenizer component that do NOT depend on the C++ RAG +// Analyzer pool. These run under plain `go test` (no -tags integration). +// Pool-dependent tests live in tokenizer_test.go (//go:build integration). + +package component + +import ( + "context" + "strings" + "sync/atomic" + "testing" + "time" + + "ragflow/internal/agent/runtime" + "ragflow/internal/ingestion/component/schema" +) + +// stubEmbedder records every call and returns canned vectors. +// Matches the Embedder contract: len(results) == len(texts). +type stubEmbedder struct { + calls atomic.Int32 + dim int + maxTokens int + delay time.Duration + err error + callInputs [][]string + resultsByCall []embeddingCallResult + callTokens []int +} + +type embeddingCallResult struct { + vectors [][]float64 + tokenCount int +} + +func (s *stubEmbedder) MaxTokens() int { + return s.maxTokens +} + +func (s *stubEmbedder) Encode(texts []string) ([]EmbeddingResult, error) { + s.calls.Add(1) + copied := append([]string(nil), texts...) + s.callInputs = append(s.callInputs, copied) + if s.delay > 0 { + time.Sleep(s.delay) + } + if s.err != nil { + return nil, s.err + } + callIdx := int(s.calls.Load()) - 1 + var cfg embeddingCallResult + if callIdx < len(s.resultsByCall) { + cfg = s.resultsByCall[callIdx] + } + out := make([]EmbeddingResult, len(texts)) + for i := range texts { + var v []float64 + if i < len(cfg.vectors) { + v = append([]float64(nil), cfg.vectors[i]...) + } else { + v = make([]float64, s.dim) + v[0] = float64(i + 1) + } + tokenCount := len(texts[i]) + if callIdx < len(s.callTokens) { + tokenCount = s.callTokens[callIdx] + } else if cfg.tokenCount > 0 { + tokenCount = cfg.tokenCount + } + out[i] = EmbeddingResult{Vector: v, TokenCount: tokenCount} + } + return out, nil +} + +// newStubEmbedder returns a stub embedder for instance-level resolver injection. +func newStubEmbedder(dim int) *stubEmbedder { + return &stubEmbedder{dim: dim} +} + +// withStubEmbedder constructs a TokenizerComponent with an instance-scoped +// stub embedder resolver. The component uses the default search_method +// (["full_text","embedding"]); callers that need a different mode construct +// the component directly via NewTokenizerComponent(NewTokenizerComponentWithResolver). +func withStubEmbedder(t *testing.T, dim int) (*TokenizerComponent, *stubEmbedder) { + t.Helper() + stub := newStubEmbedder(dim) + comp, err := NewTokenizerComponentWithResolver(nil, func(_, _, _ string) (Embedder, error) { return stub, nil }) + if err != nil { + t.Fatalf("NewTokenizerComponentWithResolver: %v", err) + } + return comp.(*TokenizerComponent), stub +} + +// TestTokenizerComponent_Registered verifies init() enrollment +// under runtime.CategoryIngestion (Phase 4 / API endpoint depends +// on this contract). +func TestTokenizerComponent_Registered(t *testing.T) { + factory, cat, md, ok := runtime.DefaultRegistry.Lookup("Tokenizer") + if !ok { + t.Fatal("Tokenizer not registered in runtime.DefaultRegistry") + } + if cat != runtime.CategoryIngestion { + t.Errorf("category = %q, want %q", cat, runtime.CategoryIngestion) + } + if factory == nil { + t.Error("factory is nil") + } + if len(md.Inputs) == 0 { + t.Error("metadata.Inputs empty") + } + if len(md.Outputs) == 0 { + t.Error("metadata.Outputs empty") + } +} + +// TestTokenizerComponent_Invoke_EmptyChunks covers the no-op branch: +// empty chunk list -> empty output, no panic, no encoder call. +func TestTokenizerComponent_Invoke_EmptyChunks(t *testing.T) { + c, stub := withStubEmbedder(t, 4) + _ = stub + var err error + if err != nil { + t.Fatalf("NewTokenizerComponent: %v", err) + } + + out, err := c.Invoke(context.Background(), map[string]any{ + "output_format": "chunks", + "chunks": []map[string]any{}, + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + chunks, _ := out["chunks"].([]map[string]any) + if len(chunks) != 0 { + t.Errorf("chunks len = %d, want 0", len(chunks)) + } + if stub.calls.Load() != 0 { + t.Errorf("embedder called %d times on empty input, want 0", stub.calls.Load()) + } + if got := out["embedding_token_consumption"]; got != 0 { + t.Errorf("embedding_token_consumption = %v, want 0", got) + } + if out["output_format"] != "chunks" { + t.Errorf("output_format = %v, want chunks", out["output_format"]) + } +} + +// TestTokenizerComponent_Invoke_NilChunks covers the nil-input +// branch: nil chunks list is treated as zero-length (matches +// python `kwargs.get("chunks")` with None). +func TestTokenizerComponent_Invoke_NilChunks(t *testing.T) { + c, stub := withStubEmbedder(t, 4) + _ = stub + out, err := c.Invoke(context.Background(), map[string]any{ + "output_format": "chunks", + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + chunks, _ := out["chunks"].([]map[string]any) + if len(chunks) != 0 { + t.Errorf("chunks len = %d, want 0", len(chunks)) + } +} + +func TestTokenizerComponent_Invoke_EmbeddingOnly(t *testing.T) { + cIntf, err := NewTokenizerComponentWithResolver(map[string]any{ + "search_method": []any{"embedding"}, + }, func(_, _, _ string) (Embedder, error) { + return newStubEmbedder(4), nil + }) + if err != nil { + t.Fatalf("NewTokenizerComponentWithResolver: %v", err) + } + out, err := cIntf.(*TokenizerComponent).Invoke(context.Background(), map[string]any{ + "name": "doc.pdf", + "output_format": "chunks", + "chunks": []map[string]any{{"text": "alpha bravo"}}, + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + got, _ := out["chunks"].([]map[string]any) + if len(got) != 1 { + t.Fatalf("chunks len = %d, want 1", len(got)) + } + if got[0]["q_4_vec"] == nil { + t.Fatalf("q_4_vec missing: %v", got[0]) + } + if got[0]["content_ltks"] != nil || got[0]["content_sm_ltks"] != nil { + t.Fatalf("embedding-only mode should not emit full-text tokens: %v", got[0]) + } + if out["embedding_token_consumption"] == nil { + t.Fatal("embedding_token_consumption missing") + } +} + +// TestTokenizerComponent_Embedding_ZeroChunksStillEmitsConsumptionZero uses an +// empty chunk list, so tokenizeChunks is a no-op and the C++ pool is not needed. +func TestTokenizerComponent_Embedding_ZeroChunksStillEmitsConsumptionZero(t *testing.T) { + c, stub := withStubEmbedder(t, 2) + out, err := c.Invoke(context.Background(), map[string]any{ + "name": "doc.pdf", + "output_format": "chunks", + "chunks": []map[string]any{}, + }) + if err != nil { + t.Fatalf("Invoke: %v", err) + } + if got := stub.calls.Load(); got != 0 { + t.Fatalf("embedder calls = %d, want 0", got) + } + if got := out["embedding_token_consumption"]; got != 0 { + t.Fatalf("embedding_token_consumption = %v, want 0", got) + } +} + +// TestTokenizerComponent_InputsOutputs_NonEmpty verifies Phase 4 +// API metadata shape. +func TestTokenizerComponent_InputsOutputs_NonEmpty(t *testing.T) { + c, _ := NewTokenizerComponent(map[string]any{}) + ins := c.(*TokenizerComponent).Inputs() + outs := c.(*TokenizerComponent).Outputs() + if len(ins) == 0 { + t.Error("Inputs() empty") + } + if len(outs) == 0 { + t.Error("Outputs() empty") + } + for _, key := range []string{"chunks", "output_format"} { + if _, ok := outs[key]; !ok { + t.Errorf("Outputs() missing %q", key) + } + } + for _, key := range []string{"chunks", "name"} { + if _, ok := ins[key]; !ok { + t.Errorf("Inputs() missing %q", key) + } + } +} + +// TestTokenizerComponent_NewTokenizerComponent_Defaults verifies +// the Python default param values propagate. +func TestTokenizerComponent_NewTokenizerComponent_Defaults(t *testing.T) { + c, err := NewTokenizerComponent(nil) + if err != nil { + t.Fatalf("NewTokenizerComponent(nil): %v", err) + } + tc := c.(*TokenizerComponent) + if tc.param.FilenameEmbdWeight != 0.1 { + t.Errorf("filename_embd_weight = %v, want 0.1", tc.param.FilenameEmbdWeight) + } + if len(tc.param.Fields) != 1 || tc.param.Fields[0] != "text" { + t.Errorf("fields = %v, want [text]", tc.param.Fields) + } + if len(tc.param.SearchMethod) != 2 { + t.Errorf("search_method len = %d, want 2", len(tc.param.SearchMethod)) + } +} + +// TestTokenizerComponent_NewTokenizerComponent_BadParam covers +// the param-validation branch (invalid search_method value). +func TestTokenizerComponent_NewTokenizerComponent_BadParam(t *testing.T) { + _, err := NewTokenizerComponent(map[string]any{ + "search_method": []any{"unknown"}, + }) + if err == nil { + t.Fatal("expected param validation error, got nil") + } +} + +func TestValidateTokenizerOutputs_FullTextMissingReturnsError(t *testing.T) { + err := validateTokenizerOutputs([]schema.ChunkDoc{{Text: "alpha"}}, []string{"full_text"}, []string{"text"}) + if err == nil || !strings.Contains(err.Error(), "missing full_text tokens") { + t.Fatalf("err = %v, want missing full_text tokens", err) + } +} + +func TestValidateTokenizerOutputs_EmbeddingMissingReturnsError(t *testing.T) { + err := validateTokenizerOutputs([]schema.ChunkDoc{{Text: "alpha"}}, []string{"embedding"}, []string{"text"}) + if err == nil || !strings.Contains(err.Error(), "missing embedding vector") { + t.Fatalf("err = %v, want missing embedding vector", err) + } +} + +func TestValidateTokenizerOutputs_BothModesFailWhenOneMissing(t *testing.T) { + ck := schema.ChunkDoc{Text: "alpha", ContentLtks: "tok", ContentSmLtks: "sm"} + err := validateTokenizerOutputs([]schema.ChunkDoc{ck}, []string{"full_text", "embedding"}, []string{"text"}) + if err == nil || !strings.Contains(err.Error(), "missing embedding vector") { + t.Fatalf("err = %v, want missing embedding vector", err) + } +} + +func TestValidateTokenizerOutputs_SymbolOnlyContentLtksIsEmptyFails(t *testing.T) { + // Simulates a chunk whose Text is a symbol/punctuation character that + // the C++ RAGAnalyzer tokenizer cannot produce tokens for (e.g. "·", ")", "("). + // After tokenizeChunks runs, ContentLtks and ContentSmLtks remain empty, + // and validateTokenizerOutputs must detect this as a failure. + ck := schema.ChunkDoc{ + Text: ")", + ContentLtks: "", + ContentSmLtks: "", + } + err := validateTokenizerOutputs([]schema.ChunkDoc{ck}, []string{"full_text"}, []string{"text"}) + if err == nil || !strings.Contains(err.Error(), "missing full_text tokens") { + t.Fatalf("err = %v, want missing full_text tokens", err) + } +} diff --git a/internal/ingestion/pipeline/builtin_registry.go b/internal/ingestion/pipeline/builtin_registry.go new file mode 100644 index 0000000000..184948ee17 --- /dev/null +++ b/internal/ingestion/pipeline/builtin_registry.go @@ -0,0 +1,255 @@ +package pipeline + +import ( + "bytes" + "embed" + "encoding/json" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + "sync" +) + +const builtinTemplatePrefix = "ingestion_pipeline_" + +//go:embed template/ingestion_pipeline_*.json +var builtinTemplateFS embed.FS + +// BuiltinPipelineMeta is the API-facing metadata for one built-in ingestion +// pipeline template. The ParserID field is the value stored in the dataset's +// parser_id column for built-in pipelines. +type BuiltinPipelineMeta struct { + ParserID string `json:"parser_id"` + Title string `json:"title"` + Description string `json:"description,omitempty"` + Filename string `json:"filename"` + DSL map[string]any `json:"dsl"` +} + +type BuiltinPipeline struct { + BuiltinPipelineMeta + DSL map[string]any `json:"-"` +} + +type Registry struct { + templates map[string]*BuiltinPipeline + // aliases maps legacy parser_id values to their canonical builtin + // pipeline id. Example: "naive" -> "general". Aliases keep existing + // dataset rows runnable after the hardcoded parser list is removed; + // they are hidden from List/Refs so the front end only sees canonical + // templates. + aliases map[string]string + list []*BuiltinPipelineMeta +} + +var ( + defaultRegistryOnce sync.Once + defaultRegistry *Registry + defaultRegistryErr error +) + +func NewRegistryFromDir(dir string) (*Registry, error) { + return NewRegistryFromFS(os.DirFS(dir), ".") +} + +func DefaultRegistry() (*Registry, error) { + defaultRegistryOnce.Do(func() { + defaultRegistry, defaultRegistryErr = NewRegistryFromFS(builtinTemplateFS, "template") + }) + return defaultRegistry, defaultRegistryErr +} + +func NewRegistryFromFS(files fs.FS, dir string) (*Registry, error) { + entries, err := fs.ReadDir(files, dir) + if err != nil { + return nil, err + } + templates := make(map[string]*BuiltinPipeline) + list := make([]*BuiltinPipelineMeta, 0, len(entries)) + for _, entry := range entries { + if entry.IsDir() { + continue + } + name := entry.Name() + if !strings.HasPrefix(name, builtinTemplatePrefix) || !strings.HasSuffix(name, ".json") { + continue + } + raw, err := fs.ReadFile(files, filepath.Join(dir, name)) + if err != nil { + return nil, err + } + tpl, err := parseTemplate(name, raw) + if err != nil { + return nil, err + } + if _, exists := templates[tpl.ParserID]; exists { + return nil, fmt.Errorf("duplicate parser_id %q", tpl.ParserID) + } + templates[tpl.ParserID] = tpl + meta := tpl.BuiltinPipelineMeta + list = append(list, &meta) + } + sort.Slice(list, func(i, j int) bool { return list[i].ParserID < list[j].ParserID }) + return &Registry{templates: templates, aliases: builtinAliases(), list: list}, nil +} + +// builtinAliases returns the legacy parser_id -> canonical pipeline id +// mappings. "naive" is the historical default parser; its behavior now +// lives in the "general" builtin template, so dataset rows storing +// parser_id="naive" resolve to "general" at lookup time. +func builtinAliases() map[string]string { + return map[string]string{ + "naive": "general", + } +} + +// canonicalRef resolves a possibly-aliased ref to its canonical pipeline id. +// Unknown refs are returned unchanged so the caller can distinguish "alias +// target missing" from "not an alias". +func (r *Registry) canonicalRef(ref string) string { + if r == nil { + return ref + } + if canonical, ok := r.aliases[ref]; ok { + return canonical + } + return ref +} + +func (r *Registry) List() []*BuiltinPipelineMeta { + if r == nil { + return nil + } + out := make([]*BuiltinPipelineMeta, 0, len(r.list)) + for _, item := range r.list { + if item == nil { + continue + } + cp := *item + out = append(out, &cp) + } + return out +} + +func (r *Registry) Refs() []string { + if r == nil { + return nil + } + out := make([]string, 0, len(r.list)) + for _, item := range r.list { + if item == nil { + continue + } + out = append(out, item.ParserID) + } + return out +} + +func (r *Registry) Get(ref string) (*BuiltinPipeline, bool) { + if r == nil { + return nil, false + } + ref = strings.TrimSpace(ref) + tpl, ok := r.templates[r.canonicalRef(ref)] + if !ok || tpl == nil { + return nil, false + } + cp := *tpl + return &cp, true +} + +// IsValid reports whether ref names a builtin template, either directly or +// through an alias. Use this for parser_id validation so legacy values +// (e.g. "naive") stay accepted after the hardcoded allow-list is removed. +func (r *Registry) IsValid(ref string) bool { + if r == nil { + return false + } + ref = strings.TrimSpace(ref) + if ref == "" { + return false + } + _, ok := r.templates[r.canonicalRef(ref)] + return ok +} + +func parseTemplate(filename string, raw []byte) (*BuiltinPipeline, error) { + decoder := json.NewDecoder(bytes.NewReader(raw)) + decoder.UseNumber() + var data map[string]any + if err := decoder.Decode(&data); err != nil { + return nil, err + } + ref := builtinRefFromFilename(filename) + if ref == "" { + return nil, fmt.Errorf("invalid builtin filename %q", filename) + } + dsl, ok := data["dsl"].(map[string]any) + if !ok { + return nil, fmt.Errorf("builtin template %q missing dsl", filename) + } + tpl := &BuiltinPipeline{ + BuiltinPipelineMeta: BuiltinPipelineMeta{ + ParserID: ref, + Title: englishText(data["title"], ref), + Description: englishText(data["description"], ""), + Filename: filename, + DSL: dsl, + }, + DSL: dsl, + } + return tpl, nil +} + +func builtinRefFromFilename(filename string) string { + name := strings.TrimSuffix(filename, filepath.Ext(filename)) + name = strings.TrimPrefix(name, builtinTemplatePrefix) + if name == filename || name == "" { + return "" + } + return name +} + +// LoadBuiltinDSL returns the canonical DSL JSON for a builtin parser_id, +// resolving aliases transparently. "naive" returns the "general" template. +func LoadBuiltinDSL(parserID string) (string, error) { + r, err := DefaultRegistry() + if err != nil { + return "", fmt.Errorf("builtin pipeline registry: %w", err) + } + tpl, ok := r.Get(parserID) + if !ok { + return "", fmt.Errorf("unknown builtin parser_id: %s", parserID) + } + dslBytes, err := json.Marshal(tpl.DSL) + if err != nil { + return "", fmt.Errorf("marshal builtin DSL %s: %w", parserID, err) + } + return string(dslBytes), nil +} + +// englishText extracts the English text from a localized template field. +// The API must return English so it aligns with the front-end's hardcoded +// parser labels (web/src/locales/en.ts parserLabel.*); the front end does +// its own i18n and only uses this as the canonical value. When the field +// is a plain string it is returned as-is; when "en" is missing, any +// available translation is used as a last resort. +func englishText(v any, fallback string) string { + switch x := v.(type) { + case string: + return strings.TrimSpace(x) + case map[string]any: + if s, ok := x["en"].(string); ok && strings.TrimSpace(s) != "" { + return strings.TrimSpace(s) + } + for _, raw := range x { + if s, ok := raw.(string); ok && strings.TrimSpace(s) != "" { + return strings.TrimSpace(s) + } + } + } + return fallback +} diff --git a/internal/ingestion/pipeline/builtin_registry_test.go b/internal/ingestion/pipeline/builtin_registry_test.go new file mode 100644 index 0000000000..0b4c2db041 --- /dev/null +++ b/internal/ingestion/pipeline/builtin_registry_test.go @@ -0,0 +1,240 @@ +package pipeline + +import ( + "os" + "path/filepath" + "testing" +) + +func TestRegistryLoadsSummaries(t *testing.T) { + dir := t.TempDir() + mustWrite := func(name, body string) { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + mustWrite("ingestion_pipeline_general.json", `{"title":{"zh":"通用","en":"General"},"description":{"zh":"desc"},"dsl":{"components":{}}}`) + mustWrite("ingestion_pipeline_book.json", `{"title":{"en":"Book"},"dsl":{"components":{}}}`) + + r, err := NewRegistryFromDir(dir) + if err != nil { + t.Fatalf("NewRegistryFromDir: %v", err) + } + if got := len(r.List()); got != 2 { + t.Fatalf("len(List()) = %d, want 2", got) + } + tpl, ok := r.Get("general") + if !ok { + t.Fatal("expected general template") + } + // Title must be the English value so it aligns with the front-end's + // hardcoded parser labels, not a localized variant. + if tpl.Title != "General" { + t.Fatalf("title = %q, want General", tpl.Title) + } + if tpl.Description != "desc" { + t.Fatalf("description = %q, want desc (English)", tpl.Description) + } +} + +func TestRegistryRefsMatchList(t *testing.T) { + dir := t.TempDir() + mustWrite := func(name, body string) { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + mustWrite("ingestion_pipeline_general.json", `{"title":{"en":"General"},"dsl":{"components":{}}}`) + mustWrite("ingestion_pipeline_book.json", `{"title":{"en":"Book"},"dsl":{"components":{}}}`) + + r, err := NewRegistryFromDir(dir) + if err != nil { + t.Fatalf("NewRegistryFromDir: %v", err) + } + refs := r.Refs() + if len(refs) != 2 { + t.Fatalf("len(Refs()) = %d, want 2", len(refs)) + } + if refs[0] != "book" || refs[1] != "general" { + t.Fatalf("refs = %#v, want sorted builtin refs", refs) + } +} + +// TestRegistryVsHardcodedList compares the built-in registry with the hardcoded list in service/dataset.go +func TestRegistryVsHardcodedList(t *testing.T) { + r, err := DefaultRegistry() + if err != nil { + t.Fatalf("DefaultRegistry: %v", err) + } + + registryRefs := make(map[string]bool) + for _, ref := range r.Refs() { + registryRefs[ref] = true + t.Logf("Registry has: %s", ref) + } + + // Hardcoded list from service/dataset.go + hardcoded := map[string]bool{ + "naive": true, + "book": true, + "email": true, + "laws": true, + "manual": true, + "one": true, + "paper": true, + "picture": true, + "presentation": true, + "qa": true, + "resume": true, + "table": true, + "tag": true, + } + for h := range hardcoded { + t.Logf("Hardcoded has: %s", h) + } + + // Find differences + onlyInRegistry := []string{} + onlyInHardcoded := []string{} + inBoth := []string{} + + for ref := range registryRefs { + if hardcoded[ref] { + inBoth = append(inBoth, ref) + } else { + onlyInRegistry = append(onlyInRegistry, ref) + } + } + for h := range hardcoded { + if !registryRefs[h] { + onlyInHardcoded = append(onlyInHardcoded, h) + } + } + + t.Logf("--- Summary ---") + t.Logf("In both: %v", inBoth) + t.Logf("Only in registry: %v", onlyInRegistry) + t.Logf("Only in hardcoded: %v", onlyInHardcoded) + + // After alias mechanism: naive is an alias for general, so every + // hardcoded value is now reachable through the registry. + for h := range hardcoded { + if !r.IsValid(h) { + t.Errorf("hardcoded value %q is not valid in registry (not a ref and not an alias)", h) + } + } +} + +// TestRegistryAliasNaiveResolvesGeneral verifies that the legacy parser_id +// "naive" resolves to the "general" builtin template via the alias mechanism. +// This keeps existing dataset rows (which store parser_id="naive") runnable +// after the hardcoded list is removed. +func TestRegistryAliasNaiveResolvesGeneral(t *testing.T) { + r, err := DefaultRegistry() + if err != nil { + t.Fatalf("DefaultRegistry: %v", err) + } + + // "naive" is valid (alias) even though it is not in Refs(). + if !r.IsValid("naive") { + t.Error("IsValid(naive) = false, want true") + } + if !r.IsValid("general") { + t.Error("IsValid(general) = false, want true") + } + if r.IsValid("unknown-parser") { + t.Error("IsValid(unknown-parser) = true, want false") + } + + // Get("naive") returns the general template. + tpl, ok := r.Get("naive") + if !ok { + t.Fatal("Get(naive) returned false, want true") + } + if tpl.ParserID != "general" { + t.Fatalf("Get(naive).ParserID = %q, want general", tpl.ParserID) + } + if tpl.Filename != "ingestion_pipeline_general.json" { + t.Fatalf("Get(naive).Filename = %q, want ingestion_pipeline_general.json", tpl.Filename) + } + + // Refs/List must NOT include the alias - only canonical templates. + for _, ref := range r.Refs() { + if ref == "naive" { + t.Error("Refs() contains alias naive; aliases must be hidden from listing") + } + } + for _, meta := range r.List() { + if meta.ParserID == "naive" { + t.Error("List() contains alias naive; aliases must be hidden from listing") + } + } +} + +// TestLoadBuiltinDSL verifies that valid and aliased parser_ids resolve to +// real DSL content, and that unknown ids fail. +func TestLoadBuiltinDSL_Canonical(t *testing.T) { + dsl, err := LoadBuiltinDSL("general") + if err != nil { + t.Fatalf("LoadBuiltinDSL(general): %v", err) + } + if dsl == "" { + t.Fatal("LoadBuiltinDSL(general) returned empty DSL") + } + // DSL should start like a JSON object containing the canvas components. + if dsl[0] != '{' { + t.Fatalf("dsl is not JSON: %s", dsl[:80]) + } +} + +func TestLoadBuiltinDSL_NaiveAlias(t *testing.T) { + generalDSL, err := LoadBuiltinDSL("general") + if err != nil { + t.Fatalf("LoadBuiltinDSL(general): %v", err) + } + naiveDSL, err := LoadBuiltinDSL("naive") + if err != nil { + t.Fatalf("LoadBuiltinDSL(naive): %v", err) + } + if generalDSL != naiveDSL { + t.Fatal("naive alias must resolve to general DSL") + } +} + +func TestRegistryMetaIncludesDSL(t *testing.T) { + dir := t.TempDir() + mustWrite := func(name, body string) { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + mustWrite("ingestion_pipeline_general.json", `{ + "title": {"en": "General"}, + "dsl": {"components": {"File": {"obj": {"component_name": "File"}}}} + }`) + + r, err := NewRegistryFromDir(dir) + if err != nil { + t.Fatalf("NewRegistryFromDir: %v", err) + } + + general, ok := r.Get("general") + if !ok { + t.Fatal("Get(general) failed") + } + + // Verify DSL is included in the meta. + if general.DSL == nil { + t.Fatal("DSL is nil, want non-nil") + } + if _, ok := general.DSL["components"]; !ok { + t.Fatal("DSL missing components key") + } +} + +func TestLoadBuiltinDSL_UnknownFails(t *testing.T) { + _, err := LoadBuiltinDSL("nonexistent") + if err == nil { + t.Fatal("LoadBuiltinDSL(nonexistent) = nil, want error") + } +} diff --git a/internal/ingestion/pipeline/pipeline.go b/internal/ingestion/pipeline/pipeline.go index 243890f816..3e2259a628 100644 --- a/internal/ingestion/pipeline/pipeline.go +++ b/internal/ingestion/pipeline/pipeline.go @@ -29,6 +29,7 @@ import ( "ragflow/internal/common" redis2 "ragflow/internal/engine/redis" "ragflow/internal/ingestion/component/globals" + "ragflow/internal/utility" "github.com/cloudwego/eino/compose" ) @@ -78,8 +79,8 @@ func WithRunTracker(t *canvas.RunTracker) PipelineOption { } // WithRequireResume makes Run refuse to start when no checkpoint store can be -// resolved (no injected store AND no global Redis client). This is plan §6.a -// M4 方案 A: a deployment that cannot persist checkpoints must not silently +// resolved (no injected store AND no global Redis client). This is plan A: a +// deployment that cannot persist checkpoints must not silently // degrade to a non-resumable run — it must surface a clear, distinguishable // error (ErrResumeUnavailable) so the caller knows resume is unavailable. // Production ingestion wiring sets this; unit tests leave it off to exercise @@ -348,17 +349,17 @@ func (p *Pipeline) runPlain(runCtx context.Context, current map[string]any, comp if err != nil { if errors.Is(runCtx.Err(), context.Canceled) || errors.Is(runCtx.Err(), context.DeadlineExceeded) { if tracker != nil { - _ = tracker.MarkCancelled(runCtx, p.taskID) + utility.BestEffort(fmt.Sprintf("MarkCancelled for %s", p.taskID), func() error { return tracker.MarkCancelled(runCtx, p.taskID) }) } return current, fmt.Errorf("pipeline: run cancelled: %w", runCtx.Err()) } if tracker != nil { - _ = tracker.MarkFailed(runCtx, p.taskID, err.Error()) + utility.BestEffort(fmt.Sprintf("MarkFailed for %s", p.taskID), func() error { return tracker.MarkFailed(runCtx, p.taskID, err.Error()) }) } return current, fmt.Errorf("pipeline: run canvas workflow: %w", err) } if tracker != nil { - _ = tracker.MarkSucceeded(runCtx, p.taskID) + utility.BestEffort(fmt.Sprintf("MarkSucceeded for %s", p.taskID), func() error { return tracker.MarkSucceeded(runCtx, p.taskID) }) } return finalizeResult(current, out, runState), nil } @@ -395,8 +396,11 @@ func (p *Pipeline) runResumable(ctx context.Context, runCtx context.Context, cur out, invokeErr := compiled.Workflow.Invoke(runCtx, invokeInput, compose.WithCheckPointID(cpID)) if invokeErr == nil { if tracker != nil { - _ = tracker.ClearInterruptID(ctx, cpID) - _ = tracker.MarkSucceeded(ctx, cpID) + utility.BestEffort(fmt.Sprintf("ClearInterruptID for %s", p.taskID), func() error { return tracker.ClearInterruptID(ctx, cpID) }) + utility.BestEffort(fmt.Sprintf("MarkSucceeded for %s", p.taskID), func() error { return tracker.MarkSucceeded(ctx, cpID) }) + } + if store != nil { + utility.BestEffort(fmt.Sprintf("delete checkpoint for %s", p.taskID), func() error { return store.Delete(ctx, cpID) }) } return finalizeResult(current, out, runState), nil } @@ -405,14 +409,14 @@ func (p *Pipeline) runResumable(ctx context.Context, runCtx context.Context, cur if errors.Is(ctx.Err(), context.Canceled) || errors.Is(ctx.Err(), context.DeadlineExceeded) { p.cleanupCheckpoint(ctx, store, tracker, cpID) if tracker != nil { - _ = tracker.MarkCancelled(ctx, cpID) + utility.BestEffort(fmt.Sprintf("MarkCancelled for %s", p.taskID), func() error { return tracker.MarkCancelled(ctx, cpID) }) } return current, fmt.Errorf("pipeline: run cancelled: %w", ctx.Err()) } if !canvas.IsInterruptError(invokeErr) { if tracker != nil { - _ = tracker.MarkFailed(ctx, cpID, invokeErr.Error()) + utility.BestEffort(fmt.Sprintf("MarkFailed for %s", p.taskID), func() error { return tracker.MarkFailed(ctx, cpID, invokeErr.Error()) }) } return current, fmt.Errorf("pipeline: run canvas workflow: %w", invokeErr) } diff --git a/internal/ingestion/pipeline/real_storage_integration_test.go b/internal/ingestion/pipeline/real_storage_integration_test.go index e66fa4c3e8..71e5751649 100644 --- a/internal/ingestion/pipeline/real_storage_integration_test.go +++ b/internal/ingestion/pipeline/real_storage_integration_test.go @@ -63,7 +63,7 @@ func TestPipelineRun_TemplateGeneral_RealMySQLMinIO_OutputShape(t *testing.T) { componentpkg.ResolveDocumentStorageOverride = origDocResolver }) - templatePath := filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_general.json") + templatePath := filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_general.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) diff --git a/agent/templates/ingestion_pipeline_audio.json b/internal/ingestion/pipeline/template/ingestion_pipeline_audio.json similarity index 100% rename from agent/templates/ingestion_pipeline_audio.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_audio.json diff --git a/agent/templates/ingestion_pipeline_book.json b/internal/ingestion/pipeline/template/ingestion_pipeline_book.json similarity index 100% rename from agent/templates/ingestion_pipeline_book.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_book.json diff --git a/agent/templates/ingestion_pipeline_email.json b/internal/ingestion/pipeline/template/ingestion_pipeline_email.json similarity index 100% rename from agent/templates/ingestion_pipeline_email.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_email.json diff --git a/agent/templates/ingestion_pipeline_general.json b/internal/ingestion/pipeline/template/ingestion_pipeline_general.json similarity index 100% rename from agent/templates/ingestion_pipeline_general.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_general.json diff --git a/agent/templates/ingestion_pipeline_laws.json b/internal/ingestion/pipeline/template/ingestion_pipeline_laws.json similarity index 100% rename from agent/templates/ingestion_pipeline_laws.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_laws.json diff --git a/agent/templates/ingestion_pipeline_manual.json b/internal/ingestion/pipeline/template/ingestion_pipeline_manual.json similarity index 100% rename from agent/templates/ingestion_pipeline_manual.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_manual.json diff --git a/agent/templates/ingestion_pipeline_one.json b/internal/ingestion/pipeline/template/ingestion_pipeline_one.json similarity index 100% rename from agent/templates/ingestion_pipeline_one.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_one.json diff --git a/agent/templates/ingestion_pipeline_paper.json b/internal/ingestion/pipeline/template/ingestion_pipeline_paper.json similarity index 100% rename from agent/templates/ingestion_pipeline_paper.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_paper.json diff --git a/agent/templates/ingestion_pipeline_picture.json b/internal/ingestion/pipeline/template/ingestion_pipeline_picture.json similarity index 100% rename from agent/templates/ingestion_pipeline_picture.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_picture.json diff --git a/agent/templates/ingestion_pipeline_presentation.json b/internal/ingestion/pipeline/template/ingestion_pipeline_presentation.json similarity index 100% rename from agent/templates/ingestion_pipeline_presentation.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_presentation.json diff --git a/agent/templates/ingestion_pipeline_qa.json b/internal/ingestion/pipeline/template/ingestion_pipeline_qa.json similarity index 100% rename from agent/templates/ingestion_pipeline_qa.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_qa.json diff --git a/agent/templates/ingestion_pipeline_resume.json b/internal/ingestion/pipeline/template/ingestion_pipeline_resume.json similarity index 100% rename from agent/templates/ingestion_pipeline_resume.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_resume.json diff --git a/agent/templates/ingestion_pipeline_table.json b/internal/ingestion/pipeline/template/ingestion_pipeline_table.json similarity index 100% rename from agent/templates/ingestion_pipeline_table.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_table.json diff --git a/agent/templates/ingestion_pipeline_tag.json b/internal/ingestion/pipeline/template/ingestion_pipeline_tag.json similarity index 100% rename from agent/templates/ingestion_pipeline_tag.json rename to internal/ingestion/pipeline/template/ingestion_pipeline_tag.json diff --git a/internal/ingestion/pipeline/template_integration_test.go b/internal/ingestion/pipeline/template_integration_test.go index fc732445de..2b90f79b58 100644 --- a/internal/ingestion/pipeline/template_integration_test.go +++ b/internal/ingestion/pipeline/template_integration_test.go @@ -61,7 +61,7 @@ func TestPipelineRun_TemplateGeneral_RealComponents(t *testing.T) { RequireTokenizerPool(t) - templatePath := filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_general.json") + templatePath := filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_general.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) @@ -151,7 +151,7 @@ func TestPipelineRun_TemplateGeneral_RealComponents(t *testing.T) { func TestPipelineRun_TemplateOne_RealComponents(t *testing.T) { RequireTokenizerPool(t) - templatePath := filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_one.json") + templatePath := filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_one.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) @@ -241,7 +241,7 @@ func TestPipelineRun_TemplateOne_RealComponents_PDFDeepdocChunking(t *testing.T) t.Setenv("DEEPDOC_URL", "") t.Setenv("OSSDEEPDOC_URL", "") - templatePath := filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_one.json") + templatePath := filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_one.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) @@ -338,7 +338,7 @@ func TestPipelineRun_TemplateOne_RealComponents_PDFDeepdocChunking(t *testing.T) func TestPipelineRun_TemplateManual_RealComponents(t *testing.T) { RequireTokenizerPool(t) - templatePath := filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_manual.json") + templatePath := filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_manual.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) @@ -435,7 +435,7 @@ func TestPipelineRun_TemplateManual_RealComponents(t *testing.T) { func TestPipelineRun_TemplateLaws_RealComponents(t *testing.T) { RequireTokenizerPool(t) - templatePath := filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_laws.json") + templatePath := filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_laws.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) @@ -517,7 +517,7 @@ func TestPipelineRun_TemplateLaws_RealComponents(t *testing.T) { func TestPipelineRun_TemplatePaper_RealComponents(t *testing.T) { RequireTokenizerPool(t) - templatePath := filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_paper.json") + templatePath := filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_paper.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) @@ -597,7 +597,7 @@ func TestPipelineRun_TemplatePaper_RealComponents(t *testing.T) { func TestPipelineRun_TemplateBook_RealComponents(t *testing.T) { RequireTokenizerPool(t) - templatePath := filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_book.json") + templatePath := filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_book.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) @@ -688,7 +688,7 @@ func TestPipelineRun_TemplateResume_RealComponents(t *testing.T) { t.Skip("missing required env (OPENAI_API_KEY/OPENAI_BASE_URL/OPENAI_MODEL); skipping real resume extractor integration test") } - templatePath := filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_resume.json") + templatePath := filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_resume.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) @@ -787,7 +787,7 @@ func TestPipelineRun_AllIngestionTemplates_RealComponentsSmoke(t *testing.T) { content := "# Title\n\nIntro paragraph.\n\n## Section\n\nBody paragraph." docID := seedTemplateDocument(t, mem, "template-smoke.md", bucket, path, content) - files, err := filepath.Glob(filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_*.json")) + files, err := filepath.Glob(filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_*.json")) if err != nil { t.Fatalf("glob templates: %v", err) } @@ -1119,7 +1119,7 @@ func assertTokenizerTerminalChunk(t *testing.T, payload map[string]any, name, wa } func TestTemplateFixtures_AreWrappedTemplates(t *testing.T) { - path := filepath.Join(repoRootFromPipelineTest(t), "agent", "templates", "ingestion_pipeline_one.json") + path := filepath.Join(repoRootFromPipelineTest(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_one.json") raw, err := os.ReadFile(path) if err != nil { t.Fatalf("read fixture: %v", err) diff --git a/internal/ingestion/service/doc_state.go b/internal/ingestion/service/doc_state.go index 8ac39ca6d2..2ad3eefbf8 100644 --- a/internal/ingestion/service/doc_state.go +++ b/internal/ingestion/service/doc_state.go @@ -66,10 +66,15 @@ func (u *docStateUpdater) apply(r *taskpkg.PipelineResult) { // mergeDocMetadata reads existing metadata, fills in keys not already present // (existing keys are preserved, not overwritten), and writes the merged map back. +// A read failure aborts the merge: SetDocumentMetadata is a full overwrite, so +// writing with an empty baseline would destroy existing keys. func mergeDocMetadata(svc docStateSvc, docID string, metadata map[string]any) error { existing, err := svc.GetDocumentMetadataByID(docID) if err != nil { - existing = make(map[string]any) + return err + } + if existing == nil { + existing = map[string]any{} } for k, v := range metadata { if _, exists := existing[k]; !exists { diff --git a/internal/ingestion/service/execute_task_ack_test.go b/internal/ingestion/service/execute_task_ack_test.go index b942eace89..6b6a659047 100644 --- a/internal/ingestion/service/execute_task_ack_test.go +++ b/internal/ingestion/service/execute_task_ack_test.go @@ -278,27 +278,49 @@ func TestSettleMessage_NackOnNonTerminal(t *testing.T) { } } -// TestSettleMessage_NackOnPanic: if body panics, the message is still Nacked -// (terminal=false default) and the panic propagates. -func TestSettleMessage_NackOnPanic(t *testing.T) { +// TestSettleMessage_RecoversPanicAndAcksWhenTaskTerminal: if body panics, +// settleMessage must recover it (a single task's panic must not crash the +// worker process) and mark the task FAILED. With BP1 (DB truth), the DB +// showing FAILED makes the message terminal → Ack, avoiding an unnecessary +// redelivery. The panic must NOT propagate out of settleMessage. +func TestSettleMessage_RecoversPanicAndAcksWhenTaskTerminal(t *testing.T) { + db := testutil.SetupTestDB(t) + cleanup := testutil.ReplaceDBForTest(t, db) + defer cleanup() + _, _, docID, taskID := testutil.SeedTestData(t, db, + testutil.WithPipelineID("flow-1"), + testutil.WithTenantID("tenant-1"), + ) + ingestor := NewIngestor("test", 1, []string{"pdf"}) handle := &fakeTaskHandle{} - taskCtx := newAckTaskCtx(context.Background(), "task-1", "doc-1", handle) + taskCtx := newAckTaskCtx(context.Background(), taskID, docID, handle) - panicked := true + panicked := false func() { - defer func() { recover() }() + defer func() { + if r := recover(); r != nil { + panicked = true + } + }() ingestor.settleMessage(taskCtx, func(ctx context.Context) bool { panic("boom") }) - panicked = false }() - if !panicked { - t.Fatal("expected panic to propagate, got none") + if panicked { + t.Fatal("expected settleMessage to recover the panic, but it propagated out") } - if handle.nacks.Load() != 1 || handle.acks.Load() != 0 { - t.Fatalf("body=panic: expected 1 Nack/0 Ack, got acks=%d nacks=%d", handle.acks.Load(), handle.nacks.Load()) + // BP1: markFailed succeeded → DB shows FAILED (terminal) → Ack. + if handle.acks.Load() != 1 || handle.nacks.Load() != 0 { + t.Fatalf("body=panic: expected 1 Ack/0 Nack (markFailed→FAILED→DB terminal), got acks=%d nacks=%d", handle.acks.Load(), handle.nacks.Load()) + } + finalTask, err := dao.NewIngestionTaskDAO().GetByID(taskID) + if err != nil { + t.Fatalf("load final task: %v", err) + } + if finalTask.Status != common.FAILED { + t.Fatalf("final status = %s, want FAILED (panic -> markFailed)", finalTask.Status) } } @@ -340,3 +362,46 @@ func TestAckOrNack_NoOpWhenNoHandle(t *testing.T) { ingestor.ackOrNack(taskCtx, true) ingestor.ackOrNack(taskCtx, false) } + +// TestSettleMessage_DBTruthOverridesBodyReturn: even when the body returns +// false (non-terminal), if the DB shows the task in a terminal state the +// message must still be Acked. This is the DB truth that BP1 establishes: +// settlement is authoritative (DB state), the in-memory bool is advisory. +// The classic case: a panic was recovered and markFailed succeeded, so the +// task is FAILED even though the body returned false / panicked. +func TestSettleMessage_DBTruthOverridesBodyReturn(t *testing.T) { + db := testutil.SetupTestDB(t) + cleanup := testutil.ReplaceDBForTest(t, db) + defer cleanup() + _, _, docID, taskID := testutil.SeedTestData(t, db, + testutil.WithPipelineID("flow-1"), + testutil.WithTenantID("tenant-1"), + ) + + ingestor := NewIngestor("test", 1, []string{"pdf"}) + handle := &fakeTaskHandle{} + taskCtx := newAckTaskCtx(context.Background(), taskID, docID, handle) + + // body returns false AND marks the task FAILED — simulating a panic + // recovery where markFailed succeeded: the task is terminal (FAILED) + // but the body signals non-terminal. + body := func(ctx context.Context) bool { + ingestor.markFailed(taskID) + return false + } + + ingestor.settleMessage(taskCtx, body) + + // DB shows FAILED → terminal → Ack, overriding body's false. + if handle.acks.Load() != 1 || handle.nacks.Load() != 0 { + t.Fatalf("expected 1 Ack / 0 Nack (DB FAILED overrides body=false), got acks=%d nacks=%d", + handle.acks.Load(), handle.nacks.Load()) + } + finalTask, err := dao.NewIngestionTaskDAO().GetByID(taskID) + if err != nil { + t.Fatalf("load final task: %v", err) + } + if finalTask.Status != common.FAILED { + t.Fatalf("final status = %s, want FAILED (body did markFailed)", finalTask.Status) + } +} diff --git a/internal/ingestion/service/execute_task_test.go b/internal/ingestion/service/execute_task_test.go index 9937ec51c0..cb1af2e664 100644 --- a/internal/ingestion/service/execute_task_test.go +++ b/internal/ingestion/service/execute_task_test.go @@ -2,6 +2,7 @@ package service import ( "context" + "strings" "testing" "ragflow/internal/common" @@ -68,11 +69,51 @@ func TestExecuteTask_CheckpointParseFailureDoesNotKillProcess(t *testing.T) { } } -func TestDefaultRunDocumentTask_RequiresConfiguredPipelineID(t *testing.T) { +func TestDefaultRunDocumentTask_BothPipelineAndParserMissing(t *testing.T) { db := testutil.SetupTestDB(t) cleanup := testutil.ReplaceDBForTest(t, db) defer cleanup() + // Seed a document with empty parser_id so that neither pipeline_id + // nor parser_id is configured — the only case that should still fail. + _, kbID, docID, taskID := testutil.SeedTestData(t, db, + testutil.WithTenantID("tenant-1"), + testutil.WithKBID("kb-1"), + testutil.WithDocID("doc-1"), + testutil.WithTaskID("task-1"), + ) + + // Clear parser_id on the document so both identifiers are missing. + if err := db.Model(&entity.Document{}).Where("id = ?", docID).Update("parser_id", "").Error; err != nil { + t.Fatalf("clear parser_id: %v", err) + } + + ingestor := NewIngestor("test", 1, []string{"pdf"}) + err := ingestor.defaultRunDocumentTask(context.Background(), &entity.IngestionTask{ + ID: taskID, + DocumentID: docID, + DatasetID: kbID, + Status: common.RUNNING, + }) + if err == nil { + t.Fatal("expected error when neither pipeline_id nor parser_id is configured") + } + msg := err.Error() + if !strings.Contains(msg, "pipeline_id") && !strings.Contains(msg, "parser_id") { + t.Fatalf("error should mention pipeline_id/parser_id: %v", err) + } +} + +// TestDefaultRunDocumentTask_ParserIDWithoutPipelineID proceeds via the +// builtin DSL path when only parser_id is configured. The execution will +// fail downstream (no storage engine in test), but the error must NOT be +// about missing pipeline_id. +func TestDefaultRunDocumentTask_ParserIDWithoutPipelineID(t *testing.T) { + db := testutil.SetupTestDB(t) + cleanup := testutil.ReplaceDBForTest(t, db) + defer cleanup() + + // Seed with default ParserID="naive" and no PipelineID. _, kbID, docID, taskID := testutil.SeedTestData(t, db, testutil.WithTenantID("tenant-1"), testutil.WithKBID("kb-1"), @@ -87,11 +128,16 @@ func TestDefaultRunDocumentTask_RequiresConfiguredPipelineID(t *testing.T) { DatasetID: kbID, Status: common.RUNNING, }) + // The builtin path resolves naive->general from the embedded registry + // and proceeds to execute. It will fail because there is no storage + // engine available in this test — but it must NOT fail with a + // "no pipeline_id" error. if err == nil { - t.Fatal("expected error when no pipeline is configured") + t.Fatal("expected downstream error (no storage engine)") } - if err.Error() != "ingestion task task-1: no pipeline_id configured for document doc-1 or dataset kb-1" { - t.Fatalf("unexpected error: %v", err) + msg := err.Error() + if strings.Contains(msg, "no pipeline_id") { + t.Fatalf("builtin path must not fail with missing pipeline_id: %v", err) } } diff --git a/internal/ingestion/service/ingestion_service.go b/internal/ingestion/service/ingestion_service.go index e86b59757a..2f44ee5746 100644 --- a/internal/ingestion/service/ingestion_service.go +++ b/internal/ingestion/service/ingestion_service.go @@ -30,6 +30,7 @@ import ( "ragflow/internal/engine" redis2 "ragflow/internal/engine/redis" "ragflow/internal/entity" + pipelinepkg "ragflow/internal/ingestion/pipeline" taskpkg "ragflow/internal/ingestion/task" servicepkg "ragflow/internal/service" @@ -62,6 +63,7 @@ type Ingestor struct { taskChan chan *taskpkg.TaskContext workerWg sync.WaitGroup startOnce sync.Once + stopOnce sync.Once // guards close(ShutdownCh) against double-close on repeated Stop ingestionTaskSvc *servicepkg.IngestionTaskService docState *docStateUpdater @@ -80,6 +82,9 @@ type Ingestor struct { } func NewIngestor(name string, maxConcurrency int32, supportedTypes []string) *Ingestor { + if maxConcurrency <= 0 { + maxConcurrency = 1 + } ctx, cancel := context.WithCancel(context.Background()) id := utility.GenerateUUID() ingestor := &Ingestor{ @@ -322,13 +327,17 @@ func (e *Ingestor) executeTask(taskCtx *taskpkg.TaskContext) { func (e *Ingestor) markStopped(taskID string) bool { if _, err := e.ingestionTaskSvc.RequestStop(taskID); err != nil { common.Error(fmt.Sprintf("markStopped: RequestStop task %s: %v", taskID, err), err) + return false } if err := e.ingestionTaskSvc.MarkStopped(taskID); err != nil { common.Error(fmt.Sprintf("markStopped: MarkStopped task %s: %v", taskID, err), err) return false } if rc := redis2.Get(); rc != nil { - rc.Delete(fmt.Sprintf("%s-cancel", taskID)) + utility.BestEffort(fmt.Sprintf("clear cancel flag for %s", taskID), func() error { + rc.Delete(fmt.Sprintf("%s-cancel", taskID)) + return nil // Delete returns bool; the bool does not distinguish "not found" from "error" + }) } return true } @@ -351,8 +360,7 @@ func (e *Ingestor) runTask(ctx context.Context, task *entity.IngestionTask) bool case <-ctx.Done(): common.Info(fmt.Sprintf("Task %s cancelled", task.ID)) e.markCancelProgress(task) - e.markStopped(task.ID) - return true + return e.markStopped(task.ID) default: } @@ -361,6 +369,20 @@ func (e *Ingestor) runTask(ctx context.Context, task *entity.IngestionTask) bool return e.markFailed(task.ID) } + // This is a new run (IncrementRunCount succeeded). Any Redis cancel flag + // that exists now is stale — a leftover from a previous run whose + // markStopped failed to delete it. The current run's cancel is signalled + // by the DB status (STOPPING), which defaultCancelCheck falls back to + // when the Redis flag is absent. Clearing a stale flag here is safe: + // a genuine concurrent cancel sets the task to STOPPING in DB. + if rc := redis2.Get(); rc != nil { + key := fmt.Sprintf("%s-cancel", task.ID) + utility.BestEffort(fmt.Sprintf("clear stale cancel flag for %s", task.ID), func() error { + rc.Delete(key) + return nil // Delete returns bool; false may mean "key not found" or "error" + }) + } + if err := e.runDocumentTask(ctx, task); err != nil { if errors.Is(err, context.Canceled) { common.Info(fmt.Sprintf("Task %s cancelled during pipeline", task.ID)) @@ -448,18 +470,51 @@ func (e *Ingestor) settleToTerminal(taskID string) error { // settleMessage runs body under a heartbeat, then settles the MQ message. The // heartbeat is stopped (and waited on) before ack/nack — see startHeartbeat. -// terminal is derived from body's return value; on panic terminal defaults to -// false (non-terminal → Nack) so the broker redelivers after restart. +// A panic in body is recovered: the task is marked FAILED and the message is +// Nacked for redelivery, so a single task's panic never crashes the worker. +// Settlement queries the DB for the task's actual status: a terminal state +// (COMPLETED/STOPPED/FAILED) means Ack; anything else means Nack. The body's +// return value is advisory only — DB truth is authoritative (BP1). func (e *Ingestor) settleMessage(taskCtx *taskpkg.TaskContext, body func(context.Context) bool) (terminal bool) { stop := e.startHeartbeat(taskCtx) defer func() { stop() // stop heartbeat (and wait) before ack/nack + if r := recover(); r != nil { + // Recover the panic so the worker process survives. Mark the + // task FAILED so a redelivery does not re-run a poison message + // (processMessage Ack-skips an already-FAILED task); Nack for + // redelivery. The broker's redelivery limit handles deterministic + // poison messages. + common.Error(fmt.Sprintf("task %s panicked: %v", taskCtx.IngestionTask.ID, r), fmt.Errorf("%v", r)) + e.markFailed(taskCtx.IngestionTask.ID) + terminal = false + } + // Settlement authority is the DB, not the in-memory bool (BP1). + // Fall back to the in-memory bool only when the DB is unavailable. + if dbTerminal, ok := e.safeGetTerminal(taskCtx.IngestionTask.ID); ok { + terminal = dbTerminal + } e.ackOrNack(taskCtx, terminal) }() terminal = body(taskCtx.Ctx) return } +// safeGetTerminal queries the DB for the task's actual status and returns +// whether it is terminal (COMPLETED/STOPPED/FAILED). A recover guards +// against nil-DB panics in test environments — in that case (false, false) +// is returned so the caller falls back to the in-memory bool. +func (e *Ingestor) safeGetTerminal(taskID string) (terminal bool, ok bool) { + defer func() { recover() }() + task, err := e.ingestionTaskSvc.GetTask(taskID) + if err != nil { + return false, false + } + return task.Status == common.COMPLETED || + task.Status == common.STOPPED || + task.Status == common.FAILED, true +} + // ackOrNack settles the MQ message according to the terminal flag: Ack if the // task reached a durably-persisted terminal status, Nack otherwise so the // broker redelivers and resumes. A nil handle (standalone/test path) is a no-op. @@ -501,13 +556,31 @@ func (e *Ingestor) defaultCancelCheck(taskID string) bool { // next ctx.Err() check to abort and runTask to record progress=-1. The // goroutine exits when done is closed (executeTask returns). func (e *Ingestor) pollCancel(taskID string, cancel context.CancelFunc, done <-chan struct{}) { - // Check immediately so the test path (which sets cancelCheck to a func - // that returns true) does not need to wait for the first tick. - if e.cancelCheck(taskID) { - common.Info(fmt.Sprintf("Task %s cancel flag detected during polling, cancelling pipeline", taskID)) - cancel() - return + // checkOnce runs cancelCheck in a goroutine so the caller can select + // between the result and the done signal. This prevents a blocked + // cancelCheck (e.g. stuck DB call) from blocking pollCancel itself, + // which would cause executeTask's defer to deadlock on <-pollExited. + checkOnce := func() <-chan bool { + result := make(chan bool, 1) + go func() { + defer func() { recover() }() // goroutine may outlive pollCancel; must not crash process + result <- e.cancelCheck(taskID) + }() + return result } + + // Initial check (immediately, for the test path). + select { + case <-done: + return + case ok := <-checkOnce(): + if ok { + common.Info(fmt.Sprintf("Task %s cancel flag detected during polling, cancelling pipeline", taskID)) + cancel() + return + } + } + ticker := time.NewTicker(3 * time.Second) defer ticker.Stop() for { @@ -515,9 +588,15 @@ func (e *Ingestor) pollCancel(taskID string, cancel context.CancelFunc, done <-c case <-done: return case <-ticker.C: - if e.cancelCheck(taskID) { - cancel() + select { + case <-done: return + case ok := <-checkOnce(): + if ok { + common.Info(fmt.Sprintf("Task %s cancel flag detected during polling, cancelling pipeline", taskID)) + cancel() + return + } } } } @@ -620,18 +699,38 @@ func (e *Ingestor) defaultRunDocumentTask(ctx context.Context, ingestionTask *en if err != nil { return fmt.Errorf("load task context for %s: %w", ingestionTask.ID, err) } - if docTaskCtx.PipelineID == "" { - return fmt.Errorf("ingestion task %s: no pipeline_id configured for document %s or dataset %s", ingestionTask.ID, docTaskCtx.Doc.ID, docTaskCtx.KB.ID) + + pipelineID := strings.TrimSpace(docTaskCtx.PipelineID) + parserID := strings.TrimSpace(docTaskCtx.Doc.ParserID) + isBuiltin := pipelineID == "" + + if pipelineID == "" { + if parserID == "" { + return fmt.Errorf("ingestion task %s: no pipeline_id or parser_id configured for document %s", ingestionTask.ID, docTaskCtx.Doc.ID) + } + pipelineID = parserID // builtin: parser_id acts as the logical pipeline identifier } + docTaskCtx.Ctx = ctx // The sink owns all document/ingestion_task_log/ingestion_task.component_total // writes for this run; inject it into the executor so the pipeline reports // progress to the service layer instead of touching the DAO directly. - executor, err := taskpkg.NewPipelineExecutor(docTaskCtx, strings.TrimSpace(docTaskCtx.PipelineID), 0) + executor, err := taskpkg.NewPipelineExecutor(docTaskCtx, pipelineID, 0) if err != nil { return err } - result, err := executor.WithProgressSink(newProgressSink(e.ingestionTaskSvc)).Execute(docTaskCtx.Ctx) + if isBuiltin { + // Builtin path: load DSL from the embedded registry, skipping canvas DB lookup. + executor.WithLoadDSLFunc(func(ctx context.Context, _ string) (string, string, error) { + common.Info(fmt.Sprintf("load built in DSL for: %s", parserID)) + dsl, lerr := pipelinepkg.LoadBuiltinDSL(parserID) + if lerr != nil { + return "", "", lerr + } + return dsl, parserID, nil + }) + } + result, err := executor.WithRequireResume().WithProgressSink(newProgressSink(e.ingestionTaskSvc)).Execute(docTaskCtx.Ctx) if err != nil { return err } @@ -668,4 +767,9 @@ func (e *Ingestor) Stop(ctx context.Context) { e.tasksMu.RUnlock() common.Warn(fmt.Sprintf("Stop timed out with %d task(s) still in-flight (will be redelivered by broker): %v", len(ids), ids)) } + + // Signal shutdown completion so the cmd-side select on <-ShutdownCh + // unblocks (the admin graceful-shutdown path). Guarded by stopOnce: a + // repeated Stop must not double-close the channel. + e.stopOnce.Do(func() { close(e.ShutdownCh) }) } diff --git a/internal/ingestion/service/ingestor_lifecycle_test.go b/internal/ingestion/service/ingestor_lifecycle_test.go index 83f047efa6..d1b98a82d6 100644 --- a/internal/ingestion/service/ingestor_lifecycle_test.go +++ b/internal/ingestion/service/ingestor_lifecycle_test.go @@ -84,6 +84,22 @@ func TestStop_GracefulShutdown(t *testing.T) { } } +// TestStop_ClosesShutdownCh verifies that Stop closes ShutdownCh so the +// cmd-side select on <-ingestor.ShutdownCh unblocks and the orchestrator +// knows shutdown completed. Mirrors syncer.go which closes its ShutdownCh in +// Stop. Without this, the admin graceful-shutdown path is dead (cmd blocks +// forever on the receive). +func TestStop_ClosesShutdownCh(t *testing.T) { + ingestor := NewIngestor("test-shutdown-ch", 1, nil) + ingestor.Stop(context.Background()) + select { + case <-ingestor.ShutdownCh: + // closed - pass + default: + t.Fatal("ShutdownCh should be closed after Stop returns") + } +} + // TestStop_TimesOutWhenWorkerStuck verifies the B1 fix: when a worker is // blocked in a stage that does not honor ctx cancellation (e.g. a native // CGO parse), Stop returns once its deadline expires instead of hanging on @@ -145,3 +161,42 @@ func TestStop_TimesOutWhenWorkerStuck(t *testing.T) { close(release) ingestor.workerWg.Wait() } + +// TestPollCancel_ExitsWhenDoneClosed verifies that closing the done channel +// causes pollCancel to return even when cancelCheck is blocked (e.g. on a +// long DB query). Without BP3, the initial cancelCheck call runs +// synchronously and pollCancel cannot observe done until it returns. +func TestPollCancel_ExitsWhenDoneClosed(t *testing.T) { + ingestor := NewIngestor("test", 1, []string{"pdf"}) + + // Block cancelCheck until released — simulate a stuck DB call. + blocking := make(chan struct{}) + released := make(chan struct{}) + ingestor.cancelCheck = func(taskID string) bool { + close(blocking) + <-released + return false + } + + done := make(chan struct{}) + exited := make(chan struct{}) + go func() { + ingestor.pollCancel("task-1", func() {}, done) + close(exited) + }() + + // Wait for cancelCheck to enter the blocking call. + <-blocking + + // Close done — pollCancel must exit even though cancelCheck is stuck. + close(done) + + select { + case <-exited: + // pollCancel returned — BP3 fix works. + case <-time.After(2 * time.Second): + t.Fatal("pollCancel did not exit when done closed (stuck in blocking cancelCheck)") + } + + close(released) // cleanup +} diff --git a/internal/ingestion/task/chunk_index_writer.go b/internal/ingestion/task/chunk_index_writer.go index 72cc653175..f5ce30c372 100644 --- a/internal/ingestion/task/chunk_index_writer.go +++ b/internal/ingestion/task/chunk_index_writer.go @@ -62,6 +62,9 @@ func (w *chunkIndexWriter) Write(ctx context.Context, chunks []map[string]any) e if end > len(chunks) { end = len(chunks) } + if err := ctx.Err(); err != nil { + return err + } if _, err := w.insertFunc(ctx, chunks[b:end], w.baseName, w.datasetID); err != nil { return err } diff --git a/internal/ingestion/task/chunk_process.go b/internal/ingestion/task/chunk_process.go index 4ba8d591a6..4408522dcd 100644 --- a/internal/ingestion/task/chunk_process.go +++ b/internal/ingestion/task/chunk_process.go @@ -18,85 +18,12 @@ package task import ( "fmt" - "regexp" - "strings" "time" "ragflow/internal/common" - "ragflow/internal/tokenizer" "ragflow/internal/utility" - - "github.com/pkoukk/tiktoken-go" ) -var keywordsSplitRE = regexp.MustCompile(`[,，;；、\r\n]+`) - -// TruncateTexts truncates each text by token count using cl100k_base encoding. -// maxLength is reduced by 10 as a safety margin, matching Python. -// Mirrors Python: EmbeddingUtils.truncate_texts() -func TruncateTexts(texts []string, maxLength int) []string { - if texts == nil { - return nil - } - safeMax := maxLength - 10 - if safeMax < 0 { - safeMax = 0 - } - enc, err := tiktoken.GetEncoding("cl100k_base") - if err != nil { - // Fallback: if tiktoken fails, return as-is. - // NOTE: this path cannot be triggered in unit tests because - // tiktoken.GetEncoding always succeeds in normal environments. - // The fallback is simple (make+copy) and verified by code review. - result := make([]string, len(texts)) - copy(result, texts) - return result - } - result := make([]string, len(texts)) - for i, t := range texts { - tokens := enc.Encode(t, nil, nil) - if len(tokens) > safeMax { - result[i] = enc.Decode(tokens[:safeMax]) - } else { - result[i] = t - } - } - return result -} - -// SplitQuestions splits a questions string by newline, keeping all elements. -// Mirrors Python: ck["questions"].split("\n") — keeps empty strings -func SplitQuestions(questions string) []string { - return strings.Split(questions, "\n") -} - -// SplitKeywords splits a keywords string by common delimiters. -// Mirrors Python: re.split(r"[,，;；、\r\n]+", keywords) -func SplitKeywords(keywords string) []string { - if keywords == "" { - return nil - } - parts := keywordsSplitRE.Split(keywords, -1) - result := make([]string, 0, len(parts)) - for _, p := range parts { - if p != "" { - result = append(result, p) - } - } - if len(result) == 0 { - return nil - } - return result -} - -// CreateChunkTime returns the current timestamp as a formatted string and float Unix timestamp. -// The float has sub-second precision, matching Python: datetime.now().timestamp() -func CreateChunkTime() (string, float64) { - now := time.Now() - timeStr := now.Format("2006-01-02 15:04:05") - return timeStr, float64(now.UnixMicro()) / 1e6 -} - // RenameTextToContentWithWeight renames the "text" key to "content_with_weight". // If "content_with_weight" already exists, the "text" key is simply removed. // Mirrors Python: ck["content_with_weight"] = ck["text"]; del ck["text"] @@ -127,16 +54,21 @@ func GetEmbeddingTokenConsumption(output map[string]any) int { } // ProcessChunksForPipeline mutates chunks into the pre-index structure used by -// the pipeline and returns merged metadata. +// the pipeline and returns merged metadata. It returns an error if a chunk's +// "text" field is present but not a string: that is an upstream contract +// violation (the chunker/parser must emit string text), and continuing would +// collapse every such chunk onto the same empty-text ChunkID, silently +// overwriting each other in the index. The caller fails the task so the +// violation surfaces instead of corrupting the index. func ProcessChunksForPipeline( chunks []map[string]any, docID string, kbID string, docName string, now time.Time, -) map[string]any { +) (map[string]any, error) { if chunks == nil { - return nil + return nil, nil } metadata := make(map[string]any) timeStr := now.Format("2006-01-02 15:04:05") @@ -150,22 +82,20 @@ func ProcessChunksForPipeline( ck["create_timestamp_flt"] = timestamp if _, exists := ck["id"]; !exists { - text, err := MustGetChunkTextString(ck) + text, err := GetChunkTextString(ck) if err != nil { - common.Error("unexpected error", err) + return nil, fmt.Errorf("process chunks for pipeline: doc=%s: %w", docID, err) } ck["id"] = ChunkID(text, docID) } - processChunkQuestions(ck) - processChunkKeywords(ck) - processChunkSummary(ck) + cleanupConsumedChunkFields(ck) metadata = mergeChunkMetadata(metadata, ck) RenameTextToContentWithWeight(ck) processChunkPositions(ck) removeInternalChunkFields(ck) } - return metadata + return metadata, nil } func removeInternalChunkFields(ck map[string]any) { @@ -173,59 +103,27 @@ func removeInternalChunkFields(ck map[string]any) { delete(ck, "image") } -func processChunkQuestions(ck map[string]any) { - if _, exists := ck["questions"]; !exists { - return - } - if _, hasTks := ck["question_tks"]; !hasTks { - q, _ := ck["questions"].(string) - ck["question_kwd"] = strings.Split(q, "\n") - tks, err := tokenizer.Tokenize(q) - if err == nil { - ck["question_tks"] = tks - } else { - ck["question_tks"] = q +// cleanupConsumedChunkFields materializes the stored array form of the +// questions/keywords fields (when the upstream Tokenizer did not already set +// them) and strips the consumed source fields (questions/keywords/summary) +// before persist. This is persist-schema mapping, NOT linguistic tokenization: +// question_tks / important_tks / content_ltks are owned by the Tokenizer +// component; the executor no longer falls back to producing them. +func cleanupConsumedChunkFields(ck map[string]any) { + if q, ok := ck["questions"].(string); ok { + if _, has := ck["question_kwd"]; !has { + ck["question_kwd"] = utility.SplitQuestions(q) } } delete(ck, "questions") -} -func processChunkKeywords(ck map[string]any) { - if _, exists := ck["keywords"]; !exists { - return - } - if _, hasTks := ck["important_tks"]; !hasTks { - kws, _ := ck["keywords"].(string) - ck["important_kwd"] = SplitKeywords(kws) - tks, err := tokenizer.Tokenize(kws) - if err == nil { - ck["important_tks"] = tks - } else { - ck["important_tks"] = kws + if kws, ok := ck["keywords"].(string); ok { + if _, has := ck["important_kwd"]; !has { + ck["important_kwd"] = utility.SplitKeywords(kws) } } delete(ck, "keywords") -} -func processChunkSummary(ck map[string]any) { - if _, exists := ck["summary"]; !exists { - return - } - if _, hasLtks := ck["content_ltks"]; !hasLtks { - smmry, _ := ck["summary"].(string) - ltks, err := tokenizer.Tokenize(smmry) - if err == nil { - ck["content_ltks"] = ltks - } else { - ck["content_ltks"] = smmry - } - smLtks, err := tokenizer.FineGrainedTokenize(ck["content_ltks"].(string)) - if err == nil { - ck["content_sm_ltks"] = smLtks - } else { - ck["content_sm_ltks"] = ck["content_ltks"].(string) - } - } delete(ck, "summary") } @@ -241,13 +139,35 @@ func mergeChunkMetadata(metadata map[string]any, ck map[string]any) map[string]a return metadata } +// processChunkPositions converts the raw "positions" field into indexable +// position fields (page_num_int, top_int, position_int) via AddPositions, +// then removes the raw field. +// +// Two source types reach this point: +// - []float64 — flat array of 5-tuples [page,left,right,top,bottom,…] from +// parsers that emit positions directly as a flat float64 slice. +// - [][]float64 — the production path: positions flow through ChunkDoc +// (json.RawMessage → decodeStructuredValue) which produces a slice of +// 5-element groups. +// +// Both are flattened into a single []float64 for AddPositions, which groups +// by 5 internally. Unexpected types are logged and discarded. func processChunkPositions(ck map[string]any) { poss, exists := ck["positions"] if !exists { return } - if positions, ok := poss.([]float64); ok { - AddPositions(ck, positions) + switch v := poss.(type) { + case []float64: + AddPositions(ck, v) + case [][]float64: + flat := make([]float64, 0, len(v)*5) + for _, group := range v { + flat = append(flat, group...) + } + AddPositions(ck, flat) + default: + common.Warn(fmt.Sprintf("chunk positions unexpected type %T; discarding", poss)) } delete(ck, "positions") } diff --git a/internal/ingestion/task/chunk_process_test.go b/internal/ingestion/task/chunk_process_test.go index 33b5451ea4..176a9babc9 100644 --- a/internal/ingestion/task/chunk_process_test.go +++ b/internal/ingestion/task/chunk_process_test.go @@ -1,173 +1,12 @@ package task import ( - "math" "testing" "time" ) // ============================================================================= -// TruncateTexts — Python: tiktoken.cl100k_base token-level truncation -// ============================================================================= - -func TestTruncateTexts_TokenLevel(t *testing.T) { - // Python: enc.encode("hello world") → [15339, 1917] (2 tokens) - // With maxLength=12, safeMax=2, both tokens fit → unchanged - result := TruncateTexts([]string{"hello world"}, 12) - if len(result) != 1 { - t.Fatalf("len = %d, want 1", len(result)) - } - if result[0] != "hello world" { - t.Errorf("with safeMax=2, 'hello world' (2 tokens) should fit, got %q", result[0]) - } -} - -func TestTruncateTexts_TruncatesByToken(t *testing.T) { - // Python: with safeMax=1, keep only 1 token → shorter than original - result := TruncateTexts([]string{"hello world"}, 11) - // safeMax = 11-10 = 1, keeps only 1 token - if len(result[0]) >= len("hello world") || len(result[0]) == 0 { - t.Errorf("safeMax=1 should produce shorter output, got %q", result[0]) - } -} - -func TestTruncateTexts_EmptyText(t *testing.T) { - result := TruncateTexts([]string{""}, 100) - if len(result) != 1 || result[0] != "" { - t.Errorf("empty text should remain empty, got %q", result[0]) - } -} - -func TestTruncateTexts_NilInput(t *testing.T) { - result := TruncateTexts(nil, 100) - if result != nil { - t.Errorf("expected nil for nil input, got %v", result) - } -} - -func TestTruncateTexts_EmptySlice(t *testing.T) { - result := TruncateTexts([]string{}, 100) - if len(result) != 0 { - t.Errorf("expected empty slice, got len=%d", len(result)) - } -} - -func TestTruncateTexts_MultipleTexts(t *testing.T) { - texts := []string{"hello world", "short"} - result := TruncateTexts(texts, 100) - if len(result) != 2 { - t.Fatalf("len = %d, want 2", len(result)) - } - if result[0] != "hello world" { - t.Errorf("first text should be unchanged") - } - if result[1] != "short" { - t.Errorf("second text should be unchanged") - } -} - -// ============================================================================= -// SplitQuestions — Python: str.split("\n"), keeps empty strings -// ============================================================================= - -func TestSplitQuestions_Basic(t *testing.T) { - result := SplitQuestions("Q1\nQ2\nQ3") - if len(result) != 3 { - t.Fatalf("len = %d, want 3", len(result)) - } - if result[0] != "Q1" || result[1] != "Q2" || result[2] != "Q3" { - t.Errorf("got %v, want [Q1 Q2 Q3]", result) - } -} - -func TestSplitQuestions_Empty(t *testing.T) { - result := SplitQuestions("") - // Python: "".split("\n") → [""] - if len(result) != 1 || result[0] != "" { - t.Errorf("got %v, want [\"\"]", result) - } -} - -func TestSplitQuestions_TrailingNewline(t *testing.T) { - result := SplitQuestions("Q1\nQ2\n") - // Python: "Q1\nQ2\n".split("\n") → ["Q1", "Q2", ""] - if len(result) != 3 { - t.Fatalf("len = %d, want 3, got %v", len(result), result) - } - if result[2] != "" { - t.Errorf("last element should be empty string, got %q", result[2]) - } -} - -func TestSplitQuestions_NoNewline(t *testing.T) { - result := SplitQuestions("single") - if len(result) != 1 || result[0] != "single" { - t.Errorf("got %v, want [single]", result) - } -} - -// ============================================================================= -// SplitKeywords — Python: re.split(r"[,，;；、\r\n]+", ...) -// ============================================================================= - -func TestSplitKeywords_Comma(t *testing.T) { - result := SplitKeywords("kw1,kw2,kw3") - if len(result) != 3 { - t.Fatalf("len = %d, want 3", len(result)) - } -} - -func TestSplitKeywords_ChineseComma(t *testing.T) { - result := SplitKeywords("kw1，kw2，kw3") - if len(result) != 3 { - t.Fatalf("len = %d, want 3", len(result)) - } -} - -func TestSplitKeywords_MixedDelimiters(t *testing.T) { - result := SplitKeywords("kw1,kw2；kw3") - if len(result) != 3 { - t.Fatalf("len = %d, want 3, got %v", len(result), result) - } -} - -func TestSplitKeywords_FiltersEmptyStrings(t *testing.T) { - result := SplitKeywords("kw1,,kw2") - // Python: re.split → ["kw1", "", "kw2"] → after filter: ["kw1", "kw2"] - if len(result) != 2 { - t.Errorf("empty strings should be filtered: got %v", result) - } -} - -func TestSplitKeywords_Empty(t *testing.T) { - result := SplitKeywords("") - // Python: re.split on "" → [""], then filtered by if k.strip() - if len(result) != 0 { - t.Errorf("got %v, want empty", result) - } -} - -// ============================================================================= -// CreateChunkTime — Python: datetime.now().timestamp() has sub-second precision -// ============================================================================= - -func TestCreateChunkTime_Format(t *testing.T) { - timeStr, ts := CreateChunkTime() - if timeStr == "" { - t.Error("create_time should not be empty") - } - if len(timeStr) != 19 { - t.Errorf("expected 19 chars (2006-01-02 15:04:05), got %q", timeStr) - } - // Python: datetime.now().timestamp() returns float with fractional part - frac := ts - math.Floor(ts) - if frac <= 0 { - t.Errorf("timestamp should have sub-second precision, got frac=%f", frac) - } -} - -// ============================================================================= -// RenameTextToContentWithWeight — Python processChunks logic +// RenameTextToContentWithWeight - Python processChunks logic // ============================================================================= func TestRenameTextToContentWithWeight_Basic(t *testing.T) { @@ -201,12 +40,15 @@ func TestRenameTextToContentWithWeight_NoTextKey(t *testing.T) { } // ============================================================================= -// ProcessChunksForPipeline — Python: processChunks() +// ProcessChunksForPipeline - Python: processChunks() // ============================================================================= func TestProcessChunksForPipeline_SetsDocIDAndKBID(t *testing.T) { chunks := []map[string]any{{"text": "hello world"}} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } if chunks[0]["doc_id"] != "doc-1" { t.Errorf("doc_id = %q, want \"doc-1\"", chunks[0]["doc_id"]) @@ -222,7 +64,10 @@ func TestProcessChunksForPipeline_SetsDocIDAndKBID(t *testing.T) { func TestProcessChunksForPipeline_SetsDocNameKwd(t *testing.T) { chunks := []map[string]any{{"text": "hello"}} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } if chunks[0]["docnm_kwd"] != "test-doc.pdf" { t.Errorf("docnm_kwd = %q, want \"test-doc.pdf\"", chunks[0]["docnm_kwd"]) } @@ -231,7 +76,10 @@ func TestProcessChunksForPipeline_SetsDocNameKwd(t *testing.T) { func TestProcessChunksForPipeline_SetsTimeFields(t *testing.T) { now := time.Now() chunks := []map[string]any{{"text": "hello"}} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", now) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", now) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } if timeStr, ok := chunks[0]["create_time"].(string); ok { if timeStr != now.Format("2006-01-02 15:04:05") { @@ -252,18 +100,21 @@ func TestProcessChunksForPipeline_SetsTimeFields(t *testing.T) { func TestProcessChunksForPipeline_GeneratesID(t *testing.T) { chunks := []map[string]any{{"text": "hello"}} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } id, ok := chunks[0]["id"].(string) if !ok || id == "" { t.Errorf("id should be non-empty string, got %v", chunks[0]["id"]) } } -func TestProcessChunksForPipeline_NoPanicOnListText(t *testing.T) { +func TestProcessChunksForPipeline_ReturnsErrorOnNonStringText(t *testing.T) { chunks := []map[string]any{{"text": []any{"bad-shape"}}} - res := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) - if res == nil { - t.Errorf("should return valid result") + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err == nil { + t.Fatal("expected error when chunk text is not a string (upstream contract violation)") } } @@ -274,7 +125,10 @@ func TestProcessChunksForPipeline_RemovesInternalPipelineFields(t *testing.T) { "image": "data:image/png;base64,abc", }} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } if _, exists := chunks[0]["_pdf_positions"]; exists { t.Fatalf("_pdf_positions should be removed before indexing: %v", chunks[0]["_pdf_positions"]) } @@ -285,7 +139,10 @@ func TestProcessChunksForPipeline_RemovesInternalPipelineFields(t *testing.T) { func TestProcessChunksForPipeline_PreservesExistingID(t *testing.T) { chunks := []map[string]any{{"text": "hello", "id": "existing-id"}} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } if chunks[0]["id"] != "existing-id" { t.Errorf("existing id should be preserved, got %q", chunks[0]["id"]) } @@ -293,7 +150,10 @@ func TestProcessChunksForPipeline_PreservesExistingID(t *testing.T) { func TestProcessChunksForPipeline_QuestionsProcessing(t *testing.T) { chunks := []map[string]any{{"text": "hello", "questions": "Q1\nQ2\nQ3"}} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } if _, exists := chunks[0]["questions"]; exists { t.Error("questions key should be removed") @@ -305,14 +165,17 @@ func TestProcessChunksForPipeline_QuestionsProcessing(t *testing.T) { if len(kwd) != 3 { t.Errorf("question_kwd len = %d, want 3", len(kwd)) } - if _, ok := chunks[0]["question_tks"].(string); !ok { - t.Errorf("question_tks should be string, got %T", chunks[0]["question_tks"]) + if _, ok := chunks[0]["question_tks"]; ok { + t.Errorf("question_tks must NOT be produced by executor (owned by Tokenizer), got %T", chunks[0]["question_tks"]) } } func TestProcessChunksForPipeline_KeywordsProcessing(t *testing.T) { chunks := []map[string]any{{"text": "hello", "keywords": "kw1,kw2;kw3"}} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } if _, exists := chunks[0]["keywords"]; exists { t.Error("keywords key should be removed") @@ -321,29 +184,86 @@ func TestProcessChunksForPipeline_KeywordsProcessing(t *testing.T) { if !ok || len(kwd) == 0 { t.Errorf("important_kwd should be non-empty []string, got %v", chunks[0]["important_kwd"]) } - if _, ok := chunks[0]["important_tks"].(string); !ok { - t.Errorf("important_tks should be string, got %T", chunks[0]["important_tks"]) + if _, ok := chunks[0]["important_tks"]; ok { + t.Errorf("important_tks must NOT be produced by executor (owned by Tokenizer), got %T", chunks[0]["important_tks"]) } } func TestProcessChunksForPipeline_SummaryProcessing(t *testing.T) { chunks := []map[string]any{{"text": "hello", "summary": "This is a summary."}} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } if _, exists := chunks[0]["summary"]; exists { t.Error("summary key should be removed") } - if _, ok := chunks[0]["content_ltks"].(string); !ok { - t.Errorf("content_ltks should be string, got %T", chunks[0]["content_ltks"]) + if _, ok := chunks[0]["content_ltks"]; ok { + t.Errorf("content_ltks must NOT be produced by executor (owned by Tokenizer), got %T", chunks[0]["content_ltks"]) } - if _, ok := chunks[0]["content_sm_ltks"].(string); !ok { - t.Errorf("content_sm_ltks should be string, got %T", chunks[0]["content_sm_ltks"]) + if _, ok := chunks[0]["content_sm_ltks"]; ok { + t.Errorf("content_sm_ltks must NOT be produced by executor (owned by Tokenizer), got %T", chunks[0]["content_sm_ltks"]) + } +} + +// TestProcessChunksForPipeline_PreservesTokenizerProducedFields documents the +// Tokenizer-terminated contract: when the upstream Tokenizer already produced +// the _tks/_ltks/_kwd fields, the executor preserves them untouched and only +// strips the consumed source fields. The executor never re-tokenizes or +// overwrites Tokenizer output. +func TestProcessChunksForPipeline_PreservesTokenizerProducedFields(t *testing.T) { + chunks := []map[string]any{{ + "text": "hello", + "questions": "Q1\nQ2", + "question_tks": "tokenizer-output-tks", + "question_kwd": []string{"preset-q-kwd"}, + "keywords": "kw1,kw2", + "important_tks": "tokenizer-output-itks", + "important_kwd": []string{"preset-i-kwd"}, + "summary": "a summary", + "content_ltks": "tokenizer-output-ltks", + "content_sm_ltks": "tokenizer-output-smltks", + }} + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } + + // Consumed source fields are stripped. + for _, k := range []string{"questions", "keywords", "summary"} { + if _, exists := chunks[0][k]; exists { + t.Errorf("%s should be removed (consumed by Tokenizer)", k) + } + } + // Tokenizer-produced fields are preserved verbatim (not overwritten). + if chunks[0]["question_tks"] != "tokenizer-output-tks" { + t.Errorf("question_tks overwritten: %v", chunks[0]["question_tks"]) + } + if chunks[0]["important_tks"] != "tokenizer-output-itks" { + t.Errorf("important_tks overwritten: %v", chunks[0]["important_tks"]) + } + if chunks[0]["content_ltks"] != "tokenizer-output-ltks" { + t.Errorf("content_ltks overwritten: %v", chunks[0]["content_ltks"]) + } + if chunks[0]["content_sm_ltks"] != "tokenizer-output-smltks" { + t.Errorf("content_sm_ltks overwritten: %v", chunks[0]["content_sm_ltks"]) + } + // Preset _kwd arrays are preserved (executor does not overwrite). + if kwd, ok := chunks[0]["question_kwd"].([]string); !ok || len(kwd) != 1 || kwd[0] != "preset-q-kwd" { + t.Errorf("question_kwd preset not preserved: %v", chunks[0]["question_kwd"]) + } + if kwd, ok := chunks[0]["important_kwd"].([]string); !ok || len(kwd) != 1 || kwd[0] != "preset-i-kwd" { + t.Errorf("important_kwd preset not preserved: %v", chunks[0]["important_kwd"]) } } func TestProcessChunksForPipeline_TextRenamed(t *testing.T) { chunks := []map[string]any{{"text": "hello world"}} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } if _, exists := chunks[0]["text"]; exists { t.Error("text key should be removed") @@ -355,8 +275,56 @@ func TestProcessChunksForPipeline_TextRenamed(t *testing.T) { func TestProcessChunksForPipeline_PreservesContentWithWeight(t *testing.T) { chunks := []map[string]any{{"content_with_weight": "already set", "text": "hello"}} - _ = ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + _, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "test-doc.pdf", time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } if chunks[0]["content_with_weight"] != "already set" { t.Errorf("content_with_weight = %q, want \"already set\"", chunks[0]["content_with_weight"]) } } + +func TestProcessChunkPositions_FlatFloat64(t *testing.T) { + chunk := map[string]any{ + "positions": []float64{0, 100, 50, 200, 150}, + } + processChunkPositions(chunk) + + if _, exists := chunk["positions"]; exists { + t.Fatal("positions key must be removed") + } + pageNum := chunk["page_num_int"].([]int) + if len(pageNum) != 1 || pageNum[0] != 1 { + t.Errorf("page_num_int = %v, want [1]", pageNum) + } +} + +func TestProcessChunkPositions_2DFloat64(t *testing.T) { + chunk := map[string]any{ + "positions": [][]float64{ + {0, 100, 50, 200, 150}, + {1, 200, 60, 300, 250}, + }, + } + processChunkPositions(chunk) + + if _, exists := chunk["positions"]; exists { + t.Fatal("positions key must be removed") + } + pageNum := chunk["page_num_int"].([]int) + if len(pageNum) != 2 || pageNum[0] != 1 || pageNum[1] != 2 { + t.Errorf("page_num_int = %v, want [1 2]", pageNum) + } + top := chunk["top_int"].([]int) + if len(top) != 2 || top[0] != 200 || top[1] != 300 { + t.Errorf("top_int = %v, want [200 300]", top) + } +} + +func TestProcessChunkPositions_NoPositions(t *testing.T) { + chunk := map[string]any{"text": "hello"} + processChunkPositions(chunk) + if _, exists := chunk["page_num_int"]; exists { + t.Error("page_num_int must not be set when positions is missing") + } +} diff --git a/internal/ingestion/task/chunk_utils.go b/internal/ingestion/task/chunk_utils.go index f03a81d658..b79cbecc2e 100644 --- a/internal/ingestion/task/chunk_utils.go +++ b/internal/ingestion/task/chunk_utils.go @@ -116,7 +116,7 @@ func PrepareTextsForPipelineEmbedding(chunks []map[string]any) []string { text, _ = chunk["summary"].(string) } if text == "" { - chunkText, err := MustGetChunkTextString(chunk) + chunkText, err := GetChunkTextString(chunk) if err != nil { common.Error("chunk[text] is not string", err) } else { @@ -130,9 +130,11 @@ func PrepareTextsForPipelineEmbedding(chunks []map[string]any) []string { return texts } -// MustGetChunkTextString returns chunk["text"] when it is a string. -// Missing text is allowed and returns empty string. -func MustGetChunkTextString(chunk map[string]any) (string, error) { +// GetChunkTextString returns chunk["text"] when it is a string. +// Missing text is allowed and returns empty string. A present-but-non-string +// value is an upstream contract violation and returns an error; the caller +// decides whether to fail the task or skip the chunk. +func GetChunkTextString(chunk map[string]any) (string, error) { val, exists := chunk["text"] if !exists || val == nil { return "", nil @@ -141,9 +143,5 @@ func MustGetChunkTextString(chunk map[string]any) (string, error) { if ok { return text, nil } - - msg := fmt.Sprintf("invalid chunk text type %T, expected string, chunk=%v", val, chunk) - err := fmt.Errorf("unexpected chunk text type - not string") - common.Error(msg, err) - return "", err + return "", fmt.Errorf("invalid chunk text type %T, expected string", val) } diff --git a/internal/ingestion/task/chunk_utils_test.go b/internal/ingestion/task/chunk_utils_test.go index 4b26306190..7f42bf14aa 100644 --- a/internal/ingestion/task/chunk_utils_test.go +++ b/internal/ingestion/task/chunk_utils_test.go @@ -320,12 +320,11 @@ func TestPrepareTexts_NoPanicOnListText(t *testing.T) { } } -func TestMustGetChunkTextString_NoPanicOnStringSlice(t *testing.T) { +func TestGetChunkTextString_ReturnsErrorOnNonString(t *testing.T) { chunk := map[string]any{"text": []string{"bad-shape"}} - if _, err := MustGetChunkTextString(chunk); err == nil { - t.Errorf("expect error when chunk[text] is not string") + if _, err := GetChunkTextString(chunk); err == nil { + t.Fatal("expected error when chunk[text] is not a string") } - } func TestGetEmbeddingTokenConsumption_Int(t *testing.T) { input := map[string]any{EmbeddingTokenConsumptionKey: 42} diff --git a/internal/ingestion/task/embedder.go b/internal/ingestion/task/embedder.go index f02fcfcebd..cfcf2d7e5a 100644 --- a/internal/ingestion/task/embedder.go +++ b/internal/ingestion/task/embedder.go @@ -39,6 +39,9 @@ func (e *embedder) MaxTokens() int { } func (e *embedder) Encode(texts []string) ([]componentpkg.EmbeddingResult, error) { + if e.model.ModelDriver == nil { + return nil, fmt.Errorf("embedder: embedding model driver is nil for model %v", e.model.ModelName) + } config := &models.EmbeddingConfig{Dimension: 0} embeds, err := e.model.ModelDriver.Embed(e.model.ModelName, texts, e.model.APIConfig, config) if err != nil { diff --git a/internal/ingestion/task/golden_compare.go b/internal/ingestion/task/golden_compare.go index 106ade14a0..dc2bdd4350 100644 --- a/internal/ingestion/task/golden_compare.go +++ b/internal/ingestion/task/golden_compare.go @@ -34,14 +34,17 @@ func ProcessPipelineOutputForGolden( docID string, kbID string, docName string, -) GoldenCompareResult { +) (GoldenCompareResult, error) { normalized := NormalizeChunks(pipelineOutput) if normalized == nil { normalized = []map[string]any{} } processed := deepCopyChunks(normalized) - metadata := ProcessChunksForPipeline(processed, docID, kbID, docName, time.Now()) + metadata, err := ProcessChunksForPipeline(processed, docID, kbID, docName, time.Now()) + if err != nil { + return GoldenCompareResult{}, err + } if metadata == nil { metadata = map[string]any{} } @@ -50,5 +53,5 @@ func ProcessPipelineOutputForGolden( NormalizedChunks: normalized, ProcessedChunks: processed, MergedMetadata: metadata, - } + }, nil } diff --git a/internal/ingestion/task/golden_compare_test.go b/internal/ingestion/task/golden_compare_test.go index 0764078aa2..eb2761bdc0 100644 --- a/internal/ingestion/task/golden_compare_test.go +++ b/internal/ingestion/task/golden_compare_test.go @@ -10,7 +10,10 @@ func TestProcessPipelineOutputForGolden_Markdown(t *testing.T) { "markdown": "# Title\n\nContent", } - result := ProcessPipelineOutputForGolden(input, "doc-1", "kb-1", "sample.md") + result, err := ProcessPipelineOutputForGolden(input, "doc-1", "kb-1", "sample.md") + if err != nil { + t.Fatalf("ProcessPipelineOutputForGolden: %v", err) + } if len(result.NormalizedChunks) != 1 { t.Fatalf("normalized len = %d, want 1", len(result.NormalizedChunks)) @@ -41,7 +44,10 @@ func TestProcessChunksForPipeline_StableFields(t *testing.T) { }, } - meta := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "sample.md", now) + meta, err := ProcessChunksForPipeline(chunks, "doc-1", "kb-1", "sample.md", now) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } chunk := chunks[0] if chunk["doc_id"] != "doc-1" { diff --git a/internal/ingestion/task/pipeline_executor.go b/internal/ingestion/task/pipeline_executor.go index 4e3ff8041c..6b0bb6478b 100644 --- a/internal/ingestion/task/pipeline_executor.go +++ b/internal/ingestion/task/pipeline_executor.go @@ -52,6 +52,7 @@ type PipelineExecutor struct { loadDSLFunc func(ctx context.Context, canvasID string) (string, string, error) runPipelineFunc func(ctx context.Context, dsl string) (map[string]any, string, error) progressSink pipelinepkg.ProgressSink + requireResume bool // when true, defaultRunPipeline passes WithRequireResume to the pipeline } func validateTaskContext(taskCtx *TaskContext) error { @@ -134,6 +135,14 @@ func (s *PipelineExecutor) WithProgressSink(sink pipelinepkg.ProgressSink) *Pipe return s } +// WithRequireResume makes the pipeline refuse to start when no checkpoint +// store is resolvable (Redis down or not configured). Production ingestion +// sets this; tests skip it so they can exercise runPlain without Redis. +func (s *PipelineExecutor) WithRequireResume() *PipelineExecutor { + s.requireResume = true + return s +} + func (s *PipelineExecutor) KB() *entity.Knowledgebase { return &s.taskCtx.KB } func (s *PipelineExecutor) Doc() *entity.Document { return &s.taskCtx.Doc } func (s *PipelineExecutor) Tenant() *entity.Tenant { return &s.taskCtx.Tenant } @@ -186,13 +195,16 @@ func (s *PipelineExecutor) processOutput(ctx context.Context, pipelineOutput map } embeddingTokenConsumption := GetEmbeddingTokenConsumption(pipelineOutput) - metadata := ProcessChunksForPipeline( + metadata, err := ProcessChunksForPipeline( chunks, s.taskCtx.Doc.ID, s.taskCtx.Doc.KbID, *s.taskCtx.Doc.Name, time.Now(), ) + if err != nil { + return nil, err + } if err := s.indexWriter.Write(ctx, chunks); err != nil { return nil, err diff --git a/internal/ingestion/task/pipeline_executor_defaults.go b/internal/ingestion/task/pipeline_executor_defaults.go index 0b41062991..eb39367a03 100644 --- a/internal/ingestion/task/pipeline_executor_defaults.go +++ b/internal/ingestion/task/pipeline_executor_defaults.go @@ -61,9 +61,14 @@ func (s *PipelineExecutor) defaultRunPipeline(ctx context.Context, dsl string) ( if s.taskCtx.IngestionTask != nil && s.taskCtx.IngestionTask.ID != "" { pipelineID = s.taskCtx.IngestionTask.ID } - pipe, err := pipelinepkg.NewPipelineFromDSL([]byte(dsl), pipelineID, + opts := []pipelinepkg.PipelineOption{ pipelinepkg.WithProgressSink(s.progressSink), - pipelinepkg.WithDocumentID(s.taskCtx.Doc.ID)) + pipelinepkg.WithDocumentID(s.taskCtx.Doc.ID), + } + if s.requireResume { + opts = append(opts, pipelinepkg.WithRequireResume()) + } + pipe, err := pipelinepkg.NewPipelineFromDSL([]byte(dsl), pipelineID, opts...) if err != nil { return nil, dsl, fmt.Errorf("compile pipeline dsl: %w", err) } diff --git a/internal/ingestion/task/pipeline_executor_test.go b/internal/ingestion/task/pipeline_executor_test.go index 14ec3cc157..75475184de 100644 --- a/internal/ingestion/task/pipeline_executor_test.go +++ b/internal/ingestion/task/pipeline_executor_test.go @@ -161,7 +161,10 @@ func TestKB_Doc_Tenant_Accessors(t *testing.T) { func TestPipelineExecutor_ProcessChunks_WrapsProcessChunksForPipeline(t *testing.T) { svc := mustNewPipelineExecutor(t, makeTaskCtx(), "flow-1", 0) chunks := []map[string]any{{"text": "hello world"}} - meta := ProcessChunksForPipeline(chunks, svc.taskCtx.Doc.ID, svc.taskCtx.Doc.KbID, *svc.taskCtx.Doc.Name, time.Now()) + meta, err := ProcessChunksForPipeline(chunks, svc.taskCtx.Doc.ID, svc.taskCtx.Doc.KbID, *svc.taskCtx.Doc.Name, time.Now()) + if err != nil { + t.Fatalf("ProcessChunksForPipeline: %v", err) + } // Verify the wrapper method works correctly and chunks are processed if chunks[0]["doc_id"] != "doc-1" { diff --git a/internal/ingestion/task/pipeline_real_integration_test.go b/internal/ingestion/task/pipeline_real_integration_test.go index a31082e42b..24746eac56 100644 --- a/internal/ingestion/task/pipeline_real_integration_test.go +++ b/internal/ingestion/task/pipeline_real_integration_test.go @@ -65,7 +65,7 @@ func TestPipelineExecutor_Run_RealCanvasDSL_UsesGeneralPipeline(t *testing.T) { storage.GetStorageFactory().SetStorage(origStorage) }) - templatePath := filepath.Join(taskRepoRoot(t), "agent", "templates", "ingestion_pipeline_general.json") + templatePath := filepath.Join(taskRepoRoot(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_general.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) @@ -193,7 +193,7 @@ func TestPipelineExecutor_Run_RealPDF_WritesAndReadsBackFromElasticsearch(t *tes componentpkg.ResolveDocumentStorageOverride = origDocResolver }) - templatePath := filepath.Join(taskRepoRoot(t), "agent", "templates", "ingestion_pipeline_general.json") + templatePath := filepath.Join(taskRepoRoot(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_general.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) @@ -325,7 +325,7 @@ func TestRunPipeline_RealPipelineOutput_ProducesIndexFields(t *testing.T) { storage.GetStorageFactory().SetStorage(origStorage) }) - templatePath := filepath.Join(taskRepoRoot(t), "agent", "templates", "ingestion_pipeline_general.json") + templatePath := filepath.Join(taskRepoRoot(t), "internal", "ingestion", "pipeline", "template", "ingestion_pipeline_general.json") templateBytes, err := os.ReadFile(templatePath) if err != nil { t.Fatalf("read template: %v", err) diff --git a/internal/ingestion/task/task_context.go b/internal/ingestion/task/task_context.go index e51b98ded7..a0e2fcf276 100644 --- a/internal/ingestion/task/task_context.go +++ b/internal/ingestion/task/task_context.go @@ -59,8 +59,11 @@ func NewTaskContextForScheduling(ctx context.Context, task *entity.IngestionTask // It follows the FK chain: ingestion task -> document -> knowledgebase -> tenant. func LoadFromIngestionTask(ingestionTask *entity.IngestionTask) (*TaskContext, error) { doc, err := dao.NewDocumentDAO().GetByID(ingestionTask.DocumentID) - if err != nil || doc == nil { - return nil, fmt.Errorf("error when load document %s : %w", ingestionTask.DocumentID, err) + if err != nil { + return nil, fmt.Errorf("load document %s: %w", ingestionTask.DocumentID, err) + } + if doc == nil { + return nil, fmt.Errorf("document %s not found", ingestionTask.DocumentID) } kb, err := dao.NewKnowledgebaseDAO().GetByID(doc.KbID) diff --git a/internal/ingestion/task/tool/compare_pipeline_golden.go b/internal/ingestion/task/tool/compare_pipeline_golden.go index 9dbc6fd487..f97542d180 100644 --- a/internal/ingestion/task/tool/compare_pipeline_golden.go +++ b/internal/ingestion/task/tool/compare_pipeline_golden.go @@ -64,7 +64,7 @@ func runActual(input map[string]any, docID string, kbID string, docName string) err = fmt.Errorf("panic: %v", r) } }() - return task.ProcessPipelineOutputForGolden(input, docID, kbID, docName), nil + return task.ProcessPipelineOutputForGolden(input, docID, kbID, docName) } func reportProcessErrorOutcome(expected map[string]any, hasExpected bool, actualErr error) { diff --git a/internal/parser/parser/factory.go b/internal/parser/parser/factory.go deleted file mode 100644 index d884f1e389..0000000000 --- a/internal/parser/parser/factory.go +++ /dev/null @@ -1,84 +0,0 @@ -// -// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -package parser - -import "fmt" - -// stubParser is a placeholder for parsers whose real implementation lives -// in the DeepDoc inference service. The stub exists only to satisfy the -// factory dispatch contract. -type stubParser struct{ name string } - -func (p *stubParser) ParseWithResult(filename string, data []byte) ParseResult { - return ParseResult{Err: fmt.Errorf("%s: remote deepdoc dispatch not yet wired", p.name)} -} - -// GetParserByID returns a parser for the given parser_id (matches Python FACTORY dispatch). -// Most parser_ids handle PDF files with different strategies; the strategy is encoded -// in the returned parser implementation. Stub implementations are replaced as -// colleagues deliver real parsers. -func GetParserByID(parserID string) (ParseResultProducer, error) { - switch parserID { - case "naive", "general": - return NewNaivePDFParser(), nil - case "paper": - return NewPaperPDFParser(), nil - case "book": - return NewBookPDFParser(), nil - case "presentation": - return NewPresentationParser(), nil - case "manual": - return NewManualPDFParser(), nil - case "laws": - return NewLawsPDFParser(), nil - case "qa": - return NewQAPDFParser(), nil - case "table": - return NewTableParser(), nil - case "resume": - return NewResumePDFParser(), nil - case "picture": - return NewPictureParser(), nil - case "one": - return NewOnePDFParser(), nil - case "audio": - return NewAudioParser(), nil - case "email": - return NewEmailParser(), nil - case "tag": - return NewTagPDFParser(), nil - case "knowledge_graph": - return NewKGPDFParser(), nil - default: - return nil, fmt.Errorf("unknown parser_id: %s", parserID) - } -} - -// Stub constructors for each parser type. -func NewNaivePDFParser() *stubParser { return &stubParser{name: "naive"} } -func NewPaperPDFParser() *stubParser { return &stubParser{name: "paper"} } -func NewBookPDFParser() *stubParser { return &stubParser{name: "book"} } -func NewPresentationParser() *stubParser { return &stubParser{name: "presentation"} } -func NewManualPDFParser() *stubParser { return &stubParser{name: "manual"} } -func NewLawsPDFParser() *stubParser { return &stubParser{name: "laws"} } -func NewQAPDFParser() *stubParser { return &stubParser{name: "qa"} } -func NewTableParser() *stubParser { return &stubParser{name: "table"} } -func NewResumePDFParser() *stubParser { return &stubParser{name: "resume"} } -func NewPicturePDFParser() *stubParser { return &stubParser{name: "picture"} } -func NewOnePDFParser() *stubParser { return &stubParser{name: "one"} } -func NewTagPDFParser() *stubParser { return &stubParser{name: "tag"} } -func NewKGPDFParser() *stubParser { return &stubParser{name: "knowledge_graph"} } diff --git a/internal/parser/parser/factory_test.go b/internal/parser/parser/factory_test.go deleted file mode 100644 index 4dabdfd7a8..0000000000 --- a/internal/parser/parser/factory_test.go +++ /dev/null @@ -1,80 +0,0 @@ -// -// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -package parser - -import ( - "testing" - - "ragflow/internal/entity" -) - -func TestGetParserByID_AllKnownIDs(t *testing.T) { - tests := []struct { - parserID string - wantNil bool - }{ - // PDF-based parsers - {string(entity.ParserTypeNaive), false}, - {string(entity.ParserTypePaper), false}, - {string(entity.ParserTypeBook), false}, - {string(entity.ParserTypeManual), false}, - {string(entity.ParserTypeLaws), false}, - {string(entity.ParserTypeQA), false}, - {string(entity.ParserTypeResume), false}, - {string(entity.ParserTypePicture), false}, - {string(entity.ParserTypeOne), false}, - {string(entity.ParserTypeTag), false}, - // Office parsers - {string(entity.ParserTypePresentation), false}, - {string(entity.ParserTypeTable), false}, - // Special parsers - {string(entity.ParserTypeAudio), false}, - {string(entity.ParserTypeEmail), false}, - } - - for _, tt := range tests { - t.Run(tt.parserID, func(t *testing.T) { - got, err := GetParserByID(tt.parserID) - if tt.wantNil { - if got != nil { - t.Errorf("GetParserByID(%q) = non-nil, want nil", tt.parserID) - } - } else { - if err != nil { - t.Errorf("GetParserByID(%q): %v", tt.parserID, err) - } - if got == nil { - t.Errorf("GetParserByID(%q) = nil, want non-nil", tt.parserID) - } - } - }) - } -} - -func TestGetParserByID_InvalidID(t *testing.T) { - _, err := GetParserByID("nonexistent") - if err == nil { - t.Fatal("expected error for invalid parser_id") - } -} - -func TestGetParserByID_EmptyID(t *testing.T) { - _, err := GetParserByID("") - if err == nil { - t.Fatal("expected error for empty parser_id") - } -} diff --git a/internal/router/router.go b/internal/router/router.go index a2896006e6..33a1785bab 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -53,6 +53,7 @@ type Router struct { fileCommitHandler *handler.FileCommitHandler botHandler *handler.BotHandler componentsHandler *handler.ComponentsHandler + pipelineHandler *handler.PipelineHandler } // NewRouter create router @@ -86,6 +87,7 @@ func NewRouter( openaiChatHandler *handler.OpenAIChatHandler, botHandler *handler.BotHandler, componentsHandler *handler.ComponentsHandler, + pipelineHandler *handler.PipelineHandler, ) *Router { return &Router{ authHandler: authHandler, @@ -117,6 +119,7 @@ func NewRouter( fileCommitHandler: fileCommitHandler, botHandler: botHandler, componentsHandler: componentsHandler, + pipelineHandler: pipelineHandler, } } @@ -124,7 +127,7 @@ func NewRouter( func (r *Router) Setup(engine *gin.Engine) { SetupEERouter(engine) - + // Mark all responses from Go with a header for debugging. engine.Use(func(c *gin.Context) { c.Header("X-API-Source", "go") @@ -157,6 +160,14 @@ func (r *Router) Setup(engine *gin.Engine) { apiNoAuth.GET("/system/version", r.systemHandler.GetVersion) apiNoAuth.GET("/system/healthz", r.systemHandler.Healthz) + // Pipeline catalog. Public static data (shipped with the binary), + // no auth required. The front end uses it to populate the parser + // picker without hard-coding the parser_id list. + // Query: ?type=builtin returns built-in templates (default). + if r.pipelineHandler != nil { + apiNoAuth.GET("/pipelines", r.pipelineHandler.ListPipelines) + } + // searchbots apiNoAuth.GET("/searchbots/detail", r.searchBotHandler.SearchbotDetail) diff --git a/internal/service/dataset.go b/internal/service/dataset.go index 1945eba9f0..ed2e288770 100644 --- a/internal/service/dataset.go +++ b/internal/service/dataset.go @@ -17,18 +17,12 @@ package service import ( - "archive/zip" - "bytes" "context" - "encoding/csv" "encoding/json" - "encoding/xml" "errors" "fmt" - "io" "math" "math/rand" - "path/filepath" "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/engine" @@ -37,8 +31,8 @@ import ( enginetypes "ragflow/internal/engine/types" "ragflow/internal/entity" "ragflow/internal/entity/models" + pipelinepkg "ragflow/internal/ingestion/pipeline" "ragflow/internal/service/nlp" - "ragflow/internal/storage" "ragflow/internal/utility" "regexp" "sort" @@ -54,21 +48,6 @@ import ( ) var ( - datasetAllowedChunkMethods = map[string]struct{}{ - "naive": {}, - "book": {}, - "email": {}, - "laws": {}, - "manual": {}, - "one": {}, - "paper": {}, - "picture": {}, - "presentation": {}, - "qa": {}, - "resume": {}, - "table": {}, - "tag": {}, - } datasetSupportedAvatarMIMETypes = map[string]struct{}{ "image/jpeg": {}, "image/png": {}, @@ -83,10 +62,9 @@ var ( "time": {}, "number": {}, } - datasetChunkMethodErrorMessage = "Input should be 'naive', 'book', 'email', 'laws', 'manual', 'one', 'paper', 'picture', 'presentation', 'qa', 'resume', 'table' or 'tag'" - validIndexTypes = []string{"graph", "raptor", "mindmap"} - indexTypeToTaskType = map[string]string{"graph": "graphrag", "raptor": "raptor", "mindmap": "mindmap"} - indexTypeToDisplayName = map[string]string{"graph": "Graph", "raptor": "RAPTOR", "mindmap": "Mindmap"} + validIndexTypes = []string{"graph", "raptor", "mindmap"} + indexTypeToTaskType = map[string]string{"graph": "graphrag", "raptor": "raptor", "mindmap": "mindmap"} + indexTypeToDisplayName = map[string]string{"graph": "Graph", "raptor": "RAPTOR", "mindmap": "Mindmap"} ) const ( @@ -657,11 +635,6 @@ type embeddingCheckSample struct { QuestionKeywords []string } -type datasetParsePageRange struct { - from int64 - to int64 -} - // RunEmbedding runs embedding for all documents in a dataset. func (d *DatasetService) RunEmbedding(userID, datasetID string) (map[string]interface{}, common.ErrorCode, error) { if datasetID == "" { @@ -709,10 +682,21 @@ func (d *DatasetService) RunEmbedding(userID, datasetID string) (map[string]inte } func (d *DatasetService) runEmbeddingDocument(kb *entity.Knowledgebase, doc *entity.Document, tableDoneCountByKB map[string]int64) error { + // Determine the flow ID: pipeline_id takes precedence, parser_id acts as + // fallback (builtin DSL from registry). Both paths go through the unified + // DSL ingestion worker. + flowID := "" if doc.PipelineID != nil && strings.TrimSpace(*doc.PipelineID) != "" { - return d.queueDatasetDataflowTask(kb, doc, strings.TrimSpace(*doc.PipelineID), 0) + flowID = strings.TrimSpace(*doc.PipelineID) + } else if doc.ParserID != "" { + flowID = doc.ParserID + } + if flowID == "" { + return fmt.Errorf("document %s: no pipeline_id or parser_id configured", doc.ID) } + // Invalidate the cached column schema (field_map) when re-running table + // documents so the schema is rebuilt from the fresh parse result. if doc.ParserID == string(entity.ParserTypeTable) { doneCount, ok := tableDoneCountByKB[doc.KbID] if !ok { @@ -730,32 +714,7 @@ func (d *DatasetService) runEmbeddingDocument(kb *entity.Knowledgebase, doc *ent } } - indexName := fmt.Sprintf("ragflow_%s", kb.TenantID) - if d.docEngine != nil { - if _, err := d.docEngine.DeleteChunks(context.Background(), map[string]interface{}{"doc_id": doc.ID}, indexName, doc.KbID); err != nil { - return err - } - } - if _, err := d.taskDAO.DeleteByDocIDs([]string{doc.ID}); err != nil { - return err - } - - bucket, objectName, err := NewDocumentService().GetDocumentStorageAddress(doc) - if err != nil { - return err - } - if err := d.queueDatasetParseTasks(doc, bucket, objectName, 0); err != nil { - return err - } - if err := d.beginDatasetParseDocument(doc.ID); err != nil { - if _, delErr := d.taskDAO.DeleteByDocIDs([]string{doc.ID}); delErr != nil { - common.Warn("Failed to clean parse tasks after document state update failure", - zap.String("docID", doc.ID), - zap.Error(delErr)) - } - return err - } - return nil + return d.queueDatasetDataflowTask(kb, doc, flowID, 0) } func (d *DatasetService) queueDatasetDataflowTask(kb *entity.Knowledgebase, doc *entity.Document, flowID string, priority int64) error { @@ -801,62 +760,6 @@ func (d *DatasetService) countDoneDocuments(datasetID string) (int64, error) { return count, err } -func (d *DatasetService) queueDatasetParseTasks(doc *entity.Document, bucket, objectName string, priority int64) error { - tasks, err := d.buildDatasetParseTasks(doc, bucket, objectName, priority) - if err != nil { - return err - } - if len(tasks) == 0 { - return nil - } - if err := d.taskDAO.CreateMany(tasks); err != nil { - return err - } - queueName := datasetParseQueueName(doc, priority) - for _, task := range tasks { - if task.Progress >= 1 { - continue - } - if redisClient := redisengine.Get(); redisClient == nil || !redisClient.QueueProduct(queueName, datasetParseTaskMessage(task)) { - if _, delErr := d.taskDAO.DeleteByDocIDs([]string{doc.ID}); delErr != nil { - common.Warn("Failed to clean parse tasks after Redis enqueue failure", - zap.String("docID", doc.ID), - zap.Error(delErr)) - } - return fmt.Errorf("Can't access Redis. Please check the Redis' status.") - } - } - return nil -} - -func (d *DatasetService) buildDatasetParseTasks(doc *entity.Document, bucket, objectName string, priority int64) ([]*entity.Task, error) { - ranges, err := datasetParseTaskRanges(doc, bucket, objectName) - if err != nil { - return nil, err - } - now := time.Now() - tasks := make([]*entity.Task, 0, len(ranges)) - for _, pageRange := range ranges { - progressMsg := "" - digest := datasetParseTaskDigest(doc, pageRange.from, pageRange.to) - chunkIDs := "" - tasks = append(tasks, &entity.Task{ - ID: utility.GenerateUUID(), - DocID: doc.ID, - FromPage: pageRange.from, - ToPage: pageRange.to, - TaskType: "", - Priority: priority, - BeginAt: &now, - Progress: 0, - ProgressMsg: &progressMsg, - Digest: &digest, - ChunkIDs: &chunkIDs, - }) - } - return tasks, nil -} - func (d *DatasetService) beginDatasetParseDocument(docID string) error { now := time.Now() return dao.GetDB().Model(&entity.Document{}).Where("id = ?", docID).Updates(map[string]interface{}{ @@ -1316,335 +1219,6 @@ func datasetParseTaskMessage(task *entity.Task) map[string]interface{} { } } -func datasetParseTaskDigest(doc *entity.Document, fromPage, toPage int64) string { - hasher := xxhash.New() - config := datasetChunkingConfigForDigest(doc) - keys := make([]string, 0, len(config)) - for key := range config { - keys = append(keys, key) - } - sort.Strings(keys) - for _, key := range keys { - hasher.WriteString(datasetStableString(config[key])) - } - hasher.WriteString(doc.ID) - hasher.WriteString(strconv.FormatInt(fromPage, 10)) - hasher.WriteString(strconv.FormatInt(toPage, 10)) - return fmt.Sprintf("%x", hasher.Sum64()) -} - -func datasetChunkingConfigForDigest(doc *entity.Document) map[string]interface{} { - return map[string]interface{}{ - "doc_id": doc.ID, - "kb_id": doc.KbID, - "parser_id": doc.ParserID, - "parser_config": datasetCopyParserConfigForDigest(doc.ParserConfig), - } -} - -func datasetCopyParserConfigForDigest(config map[string]interface{}) map[string]interface{} { - copied := make(map[string]interface{}, len(config)) - for key, value := range config { - if key == "raptor" || key == "graphrag" { - continue - } - copied[key] = value - } - return copied -} - -func datasetStableString(value interface{}) string { - binary, err := json.Marshal(value) - if err != nil { - return fmt.Sprint(value) - } - return string(binary) -} - -func datasetParseTaskRanges(doc *entity.Document, bucket, objectName string) ([]datasetParsePageRange, error) { - if doc.Type == "pdf" { - return datasetPDFParseTaskRanges(doc, bucket, objectName) - } - if doc.ParserID == string(entity.ParserTypeTable) { - return datasetTableParseTaskRanges(doc, bucket, objectName) - } - return []datasetParsePageRange{{from: 0, to: maximumTaskPageNumber}}, nil -} - -func datasetPDFParseTaskRanges(doc *entity.Document, bucket, objectName string) ([]datasetParsePageRange, error) { - binary, err := datasetStorageBinary(bucket, objectName) - if err != nil { - return nil, err - } - pages := datasetEstimatePDFPageCount(binary) - pageSize := int64(datasetParserConfigInt(doc.ParserConfig, "task_page_size", 12)) - if doc.ParserID == string(entity.ParserTypePaper) { - pageSize = int64(datasetParserConfigInt(doc.ParserConfig, "task_page_size", 22)) - } - if doc.ParserID == string(entity.ParserTypeOne) || - datasetParserConfigString(doc.ParserConfig, "layout_recognize", "DeepDOC") != "DeepDOC" || - datasetParserConfigBool(doc.ParserConfig, "toc_extraction", false) { - pageSize = maximumTaskPageNumber - } - if pageSize <= 0 { - pageSize = 12 - } - - pageRanges := datasetParserConfigPageRanges(doc.ParserConfig) - ranges := make([]datasetParsePageRange, 0) - for _, configuredRange := range pageRanges { - start := configuredRange.from - 1 - if start < 0 { - start = 0 - } - end := configuredRange.to - 1 - if pages >= 0 && end > pages { - end = pages - } - for page := start; page < end; page += pageSize { - to := page + pageSize - if to > end { - to = end - } - ranges = append(ranges, datasetParsePageRange{from: page, to: to}) - } - } - if len(ranges) == 0 { - ranges = append(ranges, datasetParsePageRange{from: 0, to: maximumTaskPageNumber}) - } - return ranges, nil -} - -func datasetTableParseTaskRanges(doc *entity.Document, bucket, objectName string) ([]datasetParsePageRange, error) { - binary, err := datasetStorageBinary(bucket, objectName) - if err != nil { - return nil, err - } - rows := datasetEstimateTableRowCount(datasetDocName(doc), binary) - if rows <= 0 { - return []datasetParsePageRange{{from: 0, to: maximumTaskPageNumber}}, nil - } - ranges := make([]datasetParsePageRange, 0, (rows+2999)/3000) - for row := int64(0); row < int64(rows); row += 3000 { - to := row + 3000 - if to > int64(rows) { - to = int64(rows) - } - ranges = append(ranges, datasetParsePageRange{from: row, to: to}) - } - return ranges, nil -} - -func datasetStorageBinary(bucket, objectName string) ([]byte, error) { - storageImpl := storage.GetStorageFactory().GetStorage() - if storageImpl == nil { - return nil, fmt.Errorf("storage not initialized") - } - return storageImpl.Get(bucket, objectName) -} - -func datasetDocName(doc *entity.Document) string { - if doc == nil || doc.Name == nil { - return "" - } - return *doc.Name -} - -func datasetParserConfigInt(config map[string]interface{}, key string, fallback int) int { - value, ok := config[key] - if !ok || value == nil { - return fallback - } - switch typedValue := value.(type) { - case int: - return typedValue - case int64: - return int(typedValue) - case float64: - return int(typedValue) - case json.Number: - if intValue, err := typedValue.Int64(); err == nil { - return int(intValue) - } - case string: - if intValue, err := strconv.Atoi(strings.TrimSpace(typedValue)); err == nil { - return intValue - } - } - return fallback -} - -func datasetParserConfigString(config map[string]interface{}, key, fallback string) string { - value, ok := config[key] - if !ok || value == nil { - return fallback - } - if stringValue, ok := value.(string); ok { - return stringValue - } - return fmt.Sprint(value) -} - -func datasetParserConfigBool(config map[string]interface{}, key string, fallback bool) bool { - value, ok := config[key] - if !ok || value == nil { - return fallback - } - switch typedValue := value.(type) { - case bool: - return typedValue - case string: - switch strings.ToLower(strings.TrimSpace(typedValue)) { - case "true", "1", "yes", "on": - return true - case "false", "0", "no", "off": - return false - } - } - return fallback -} - -func datasetParserConfigPageRanges(config map[string]interface{}) []datasetParsePageRange { - defaultRanges := []datasetParsePageRange{{from: 1, to: maximumPageNumber}} - raw, ok := config["pages"] - if !ok || raw == nil { - return defaultRanges - } - rawRanges, ok := raw.([]interface{}) - if !ok || len(rawRanges) == 0 { - return defaultRanges - } - - ranges := make([]datasetParsePageRange, 0, len(rawRanges)) - for _, rawRange := range rawRanges { - rangeValues, ok := rawRange.([]interface{}) - if !ok || len(rangeValues) < 2 { - continue - } - from, okFrom := datasetToInt64(rangeValues[0]) - to, okTo := datasetToInt64(rangeValues[1]) - if okFrom && okTo && to > from { - ranges = append(ranges, datasetParsePageRange{from: from, to: to}) - } - } - if len(ranges) == 0 { - return defaultRanges - } - return ranges -} - -func datasetToInt64(value interface{}) (int64, bool) { - switch typedValue := value.(type) { - case int: - return int64(typedValue), true - case int64: - return typedValue, true - case float64: - return int64(typedValue), true - case json.Number: - intValue, err := typedValue.Int64() - return intValue, err == nil - case string: - intValue, err := strconv.ParseInt(strings.TrimSpace(typedValue), 10, 64) - return intValue, err == nil - default: - return 0, false - } -} - -var datasetPDFPagePattern = regexp.MustCompile(`/Type\s*/Page\b`) - -func datasetEstimatePDFPageCount(binary []byte) int64 { - if len(binary) == 0 { - return 0 - } - return int64(len(datasetPDFPagePattern.FindAll(binary, -1))) -} - -func datasetEstimateTableRowCount(name string, binary []byte) int { - switch strings.ToLower(filepath.Ext(name)) { - case ".xlsx": - if rows, err := datasetCountXLSXRows(binary); err == nil { - return rows - } - case ".csv", ".tsv", ".txt": - return datasetCountDelimitedRows(name, binary) - } - return 0 -} - -func datasetCountDelimitedRows(name string, binary []byte) int { - reader := csv.NewReader(bytes.NewReader(binary)) - reader.FieldsPerRecord = -1 - reader.ReuseRecord = true - if strings.EqualFold(filepath.Ext(name), ".tsv") { - reader.Comma = '\t' - } - rows := 0 - for { - _, err := reader.Read() - if err == nil { - rows++ - continue - } - if err == io.EOF { - break - } - rows += bytes.Count(binary, []byte{'\n'}) - if len(binary) > 0 && binary[len(binary)-1] != '\n' { - rows++ - } - break - } - return rows -} - -func datasetCountXLSXRows(binary []byte) (int, error) { - zipReader, err := zip.NewReader(bytes.NewReader(binary), int64(len(binary))) - if err != nil { - return 0, err - } - maxRows := 0 - for _, file := range zipReader.File { - if !strings.HasPrefix(file.Name, "xl/worksheets/") || !strings.HasSuffix(file.Name, ".xml") { - continue - } - rows, err := datasetCountWorksheetRows(file) - if err != nil { - return 0, err - } - if rows > maxRows { - maxRows = rows - } - } - return maxRows, nil -} - -func datasetCountWorksheetRows(file *zip.File) (int, error) { - reader, err := file.Open() - if err != nil { - return 0, err - } - defer reader.Close() - - decoder := xml.NewDecoder(reader) - rows := 0 - for { - token, err := decoder.Token() - if err == io.EOF { - break - } - if err != nil { - return 0, err - } - start, ok := token.(xml.StartElement) - if ok && start.Name.Local == "row" { - rows++ - } - } - return rows, nil -} - func (d *DatasetService) DeleteIndex(userID, datasetID, indexType string, wipe bool) (common.ErrorCode, error) { if !checkType(indexType) { return common.CodeArgumentError, fmt.Errorf("Invalid index type '%s'", indexType) @@ -2314,7 +1888,7 @@ func (d *DatasetService) CreateDataset(req *CreateDatasetRequest, tenantID strin } if req.ChunkMethod != nil { parserID = strings.TrimSpace(*req.ChunkMethod) - if err := validateDatasetChunkMethod(parserID); err != nil { + if err := validateParserID(parserID); err != nil { return nil, common.CodeDataError, err } pipelineID = nil @@ -2406,10 +1980,10 @@ func (d *DatasetService) CreateDataset(req *CreateDatasetRequest, tenantID strin case "chunk_method", "parser_id": parserIDValue, ok := value.(string) if !ok { - return nil, common.CodeDataError, errors.New(datasetChunkMethodErrorMessage) + return nil, common.CodeDataError, parserIDError() } parserIDValue = strings.TrimSpace(parserIDValue) - if err := validateDatasetChunkMethod(parserIDValue); err != nil { + if err := validateParserID(parserIDValue); err != nil { return nil, common.CodeDataError, err } parserID = parserIDValue @@ -2754,10 +2328,10 @@ func (d *DatasetService) UpdateDataset(datasetID, tenantID string, req UpdateDat if extParserID, ok := updates["parser_id"]; ok { parserIDValue, ok := extParserID.(string) if !ok { - return nil, common.CodeDataError, errors.New(datasetChunkMethodErrorMessage) + return nil, common.CodeDataError, parserIDError() } parserID = strings.TrimSpace(parserIDValue) - if err := validateDatasetChunkMethod(parserID); err != nil { + if err := validateParserID(parserID); err != nil { return nil, common.CodeDataError, err } parserIDProvided = true @@ -2922,7 +2496,7 @@ func datasetUpdateParserID(req UpdateDatasetRequest) (string, bool, error) { if !provided { return "", false, nil } - if err := validateDatasetChunkMethod(parserID); err != nil { + if err := validateParserID(parserID); err != nil { return "", true, err } return parserID, true, nil @@ -3557,11 +3131,47 @@ func (d *DatasetService) deleteDataset(tenantID string, kb *entity.Knowledgebase }) } -func validateDatasetChunkMethod(chunkMethod string) error { - if _, ok := datasetAllowedChunkMethods[chunkMethod]; !ok { - return errors.New(datasetChunkMethodErrorMessage) +// validateParserID validates parser_id against the built-in +// pipeline registry. The registry is the single source of truth, so the +// legacy hardcoded allow-list is gone. Legacy values (e.g. "naive") are +// accepted via registry aliases. +func validateParserID(chunkMethod string) error { + registry, err := pipelinepkg.DefaultRegistry() + if err != nil || registry == nil { + return errors.New("parser_id validation unavailable: builtin pipeline registry not loaded") } - return nil + if registry.IsValid(chunkMethod) { + return nil + } + return parserIDError() +} + +// parserIDError builds a validation error that lists the valid +// canonical parser_ids from the registry, mirroring the shape of the old +// hardcoded message but driven by the embedded templates. +func parserIDError() error { + registry, err := pipelinepkg.DefaultRegistry() + if err != nil || registry == nil { + return errors.New("invalid parser_id") + } + refs := registry.Refs() + switch len(refs) { + case 0: + return errors.New("invalid parser_id") + case 1: + return fmt.Errorf("Input should be '%s'", refs[0]) + default: + return fmt.Errorf("Input should be %s or '%s'", quoteList(refs[:len(refs)-1]), refs[len(refs)-1]) + } +} + +// quoteList renders ["a", "b"] as "'a', 'b'". +func quoteList(items []string) string { + quoted := make([]string, len(items)) + for i, v := range items { + quoted[i] = "'" + v + "'" + } + return strings.Join(quoted, ", ") } func validateDatasetAvatar(avatar string) error { diff --git a/internal/service/dataset_parser_test.go b/internal/service/dataset_parser_test.go new file mode 100644 index 0000000000..6cd104ea4b --- /dev/null +++ b/internal/service/dataset_parser_test.go @@ -0,0 +1,64 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package service + +import ( + "strings" + "testing" +) + +// TestValidateParserID_AcceptsRegistryRefs verifies that every +// canonical builtin pipeline id passes validation. +func TestValidateParserID_AcceptsRegistryRefs(t *testing.T) { + for _, id := range []string{"general", "book", "audio", "qa", "table", "tag"} { + if err := validateParserID(id); err != nil { + t.Errorf("validateParserID(%q) = %v, want nil", id, err) + } + } +} + +// TestValidateParserID_AcceptsNaiveAlias verifies the legacy +// parser_id "naive" still validates (alias for general) so existing +// dataset rows are not rejected. +func TestValidateParserID_AcceptsNaiveAlias(t *testing.T) { + if err := validateParserID("naive"); err != nil { + t.Errorf("validateParserID(naive) = %v, want nil (alias for general)", err) + } +} + +// TestValidateParserID_RejectsUnknown verifies unknown/empty +// values are rejected with an error that lists the valid options. +func TestValidateParserID_RejectsUnknown(t *testing.T) { + for _, id := range []string{"", "unknown", "NAIVE"} { + err := validateParserID(id) + if err == nil { + t.Errorf("validateParserID(%q) = nil, want error", id) + } + } + + // The error message should surface the canonical valid options so the + // caller knows what to send. "general" must appear (canonical), while + // "naive" is an alias and may or may not appear. + err := validateParserID("unknown") + if err == nil { + t.Fatal("expected error for unknown parser id") + } + msg := err.Error() + if !strings.Contains(msg, "general") { + t.Errorf("error message %q should mention general", msg) + } +} diff --git a/internal/service/document.go b/internal/service/document.go index c8d2fd8039..5dab489041 100644 --- a/internal/service/document.go +++ b/internal/service/document.go @@ -2597,22 +2597,6 @@ func (s *DocumentService) UpdateDatasetDocument(userID, datasetID, documentID st return s.toUpdateDatasetDocumentResponse(updatedDoc, metaFields), common.CodeSuccess, nil } -var allowedDocumentChunkMethods = map[string]struct{}{ - "naive": {}, - "manual": {}, - "qa": {}, - "table": {}, - "paper": {}, - "book": {}, - "laws": {}, - "presentation": {}, - "picture": {}, - "one": {}, - "knowledge_graph": {}, - "email": {}, - "tag": {}, -} - func (s *DocumentService) validateDatasetDocumentUpdate(doc *entity.Document, req *UpdateDatasetDocumentRequest, present map[string]bool) (common.ErrorCode, error) { if req == nil { return common.CodeDataError, errors.New("Invalid request payload") @@ -2638,7 +2622,7 @@ func (s *DocumentService) validateDatasetDocumentUpdate(doc *entity.Document, re return common.CodeDataError, errors.New("`chunk_method` (empty string) is not valid") } chunkMethod := strings.TrimSpace(*req.ChunkMethod) - if _, ok := allowedDocumentChunkMethods[chunkMethod]; !ok { + if err := validateParserID(chunkMethod); err != nil { return common.CodeDataError, fmt.Errorf("`chunk_method` %s doesn't exist", chunkMethod) } if doc.Type == "visual" || isPresentationFile(doc.Name) { diff --git a/internal/service/ingestion_task_service.go b/internal/service/ingestion_task_service.go index 95891241c7..da08e68ef5 100644 --- a/internal/service/ingestion_task_service.go +++ b/internal/service/ingestion_task_service.go @@ -241,12 +241,26 @@ func (s *IngestionTaskService) RequestStop(taskID string) (*entity.IngestionTask } func (s *IngestionTaskService) MarkCompleted(taskID string) error { - _, err := s.transition(taskID, common.COMPLETED) + task, err := s.GetTask(taskID) + if err != nil { + return err + } + if task.Status == common.COMPLETED || task.Status == common.STOPPED || task.Status == common.FAILED { + return nil // already terminal, idempotent — mirrors MarkStopped + } + _, err = s.transition(taskID, common.COMPLETED) return err } func (s *IngestionTaskService) MarkFailed(taskID string) error { - _, err := s.transition(taskID, common.FAILED) + task, err := s.GetTask(taskID) + if err != nil { + return err + } + if task.Status == common.FAILED || task.Status == common.COMPLETED || task.Status == common.STOPPED { + return nil // already terminal, idempotent — mirrors MarkStopped + } + _, err = s.transition(taskID, common.FAILED) return err } diff --git a/internal/service/ingestion_task_service_test.go b/internal/service/ingestion_task_service_test.go index a7deb63b06..3f0714207c 100644 --- a/internal/service/ingestion_task_service_test.go +++ b/internal/service/ingestion_task_service_test.go @@ -725,6 +725,36 @@ func TestIngestionTaskServiceMarkStoppedIdempotentOnAlreadyStopped(t *testing.T) } } +func TestIngestionTaskServiceMarkFailedIdempotentOnAlreadyTerminal(t *testing.T) { + db := setupServiceTestDB(t) + pushServiceDB(t, db) + insertTestIngestionTask(t, "task-1", "user-1", "doc-1", "kb-1") + if err := dao.DB.Model(&entity.IngestionTask{}).Where("id = ?", "task-1"). + Update("status", common.COMPLETED).Error; err != nil { + t.Fatalf("set task COMPLETED: %v", err) + } + + svc := NewIngestionTaskService() + if err := svc.MarkFailed("task-1"); err != nil { + t.Fatalf("MarkFailed on already COMPLETED task should be idempotent, got: %v", err) + } +} + +func TestIngestionTaskServiceMarkCompletedIdempotentOnAlreadyTerminal(t *testing.T) { + db := setupServiceTestDB(t) + pushServiceDB(t, db) + insertTestIngestionTask(t, "task-1", "user-1", "doc-1", "kb-1") + if err := dao.DB.Model(&entity.IngestionTask{}).Where("id = ?", "task-1"). + Update("status", common.FAILED).Error; err != nil { + t.Fatalf("set task FAILED: %v", err) + } + + svc := NewIngestionTaskService() + if err := svc.MarkCompleted("task-1"); err != nil { + t.Fatalf("MarkCompleted on already FAILED task should be idempotent, got: %v", err) + } +} + func TestDocumentServiceUpdateRunProgressMirrorsFields(t *testing.T) { db := setupServiceTestDB(t) pushServiceDB(t, db) diff --git a/internal/utility/best_effort.go b/internal/utility/best_effort.go new file mode 100644 index 0000000000..3062bb1516 --- /dev/null +++ b/internal/utility/best_effort.go @@ -0,0 +1,38 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package utility + +import ( + "fmt" + + "ragflow/internal/common" +) + +// BestEffort executes f and logs any error via Error. It never panics (nil f +// is handled) and never propagates the error — best-effort means the caller +// does not want the main flow interrupted by this failure. Use it for +// resource cleanup, soft-state deletion, and progress mirroring: operations +// whose failure must be visible (logged) but must not abort the task. +func BestEffort(label string, f func() error) { + if f == nil { + common.Error(fmt.Sprintf("best-effort %s: nil function", label), fmt.Errorf("nil function")) + return + } + if err := f(); err != nil { + common.Error(fmt.Sprintf("best-effort %s failed: %v", label, err), err) + } +} diff --git a/internal/utility/best_effort_test.go b/internal/utility/best_effort_test.go new file mode 100644 index 0000000000..8743528a4d --- /dev/null +++ b/internal/utility/best_effort_test.go @@ -0,0 +1,43 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package utility + +import ( + "errors" + "testing" +) + +func TestBestEffort_NoPanicOnSuccess(t *testing.T) { + // Must not panic when f returns nil. + BestEffort("test-success", func() error { return nil }) +} + +func TestBestEffort_NoPanicOnError(t *testing.T) { + // Must not panic when f returns an error — it should be logged, not + // propagated. + BestEffort("test-fail", func() error { return errors.New("boom") }) +} + +func TestBestEffort_NoPanicOnNilFunc(t *testing.T) { + // Must not panic when f is nil. + defer func() { + if r := recover(); r != nil { + t.Fatalf("BestEffort with nil func panicked: %v", r) + } + }() + BestEffort("test-nil", nil) +} diff --git a/internal/utility/split.go b/internal/utility/split.go new file mode 100644 index 0000000000..d3163260e0 --- /dev/null +++ b/internal/utility/split.go @@ -0,0 +1,73 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package utility + +import ( + "regexp" + "strings" +) + +// keywordsSplitRE matches the delimiters used to split a keywords string into +// the important_kwd array: ASCII and CJK comma, ASCII and CJK semicolon, the +// CJK enumeration comma (、), and newlines. Mirrors Python +// task_executor.run_dataflow:879 re.split(r"[,，;；、\r\n]+", keywords). +var keywordsSplitRE = regexp.MustCompile(`[,，;；、\r\n]+`) + +// nonEmpty drops empty strings from parts and returns nil if none remain. It is +// the shared tail of SplitKeywords and SplitQuestions: split by whatever +// delimiter, then prune empties and collapse an all-empty result to nil so a +// _kwd array is absent (nil) rather than [""]. +func nonEmpty(parts []string) []string { + out := make([]string, 0, len(parts)) + for _, p := range parts { + if p != "" { + out = append(out, p) + } + } + if len(out) == 0 { + return nil + } + return out +} + +// SplitKeywords splits a keywords string by common (ASCII + CJK) delimiters, +// dropping empty elements. Returns nil for an empty input. It is the single +// authority for materializing the important_kwd array from the keywords +// string, shared by the Tokenizer component (in-pipeline) and the executor's +// persist-schema mapping. Mirrors Python task_executor.run_dataflow:879. +func SplitKeywords(keywords string) []string { + if keywords == "" { + return nil + } + return nonEmpty(keywordsSplitRE.Split(keywords, -1)) +} + +// SplitQuestions splits a questions string by newline, dropping empty lines. +// Returns nil for an empty input. It is the authority for materializing the +// question_kwd array from the questions string. +// +// NOTE: this intentionally filters empty parts (including trailing-newline +// empties), diverging from Python's str.split("\n"), which yields [""] for an +// empty input and keeps trailing empties. Filtering aligns with SplitKeywords +// and the Infinity engine path (internal/engine/infinity/chunk.go), which all +// treat "no real question" as a nil array rather than [""]. +func SplitQuestions(s string) []string { + if s == "" { + return nil + } + return nonEmpty(strings.Split(s, "\n")) +} diff --git a/internal/utility/split_test.go b/internal/utility/split_test.go new file mode 100644 index 0000000000..6225edf666 --- /dev/null +++ b/internal/utility/split_test.go @@ -0,0 +1,96 @@ +// +// Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package utility + +import "testing" + +// SplitKeywords - Python task_executor.run_dataflow:879 +// re.split(r"[,，;；、\r\n]+", keywords) with empty filtering. + +func TestSplitKeywords_Comma(t *testing.T) { + result := SplitKeywords("kw1,kw2,kw3") + if len(result) != 3 { + t.Fatalf("len = %d, want 3", len(result)) + } +} + +func TestSplitKeywords_ChineseComma(t *testing.T) { + result := SplitKeywords("kw1，kw2，kw3") + if len(result) != 3 { + t.Fatalf("len = %d, want 3", len(result)) + } +} + +func TestSplitKeywords_MixedDelimiters(t *testing.T) { + result := SplitKeywords("kw1,kw2；kw3") + if len(result) != 3 { + t.Fatalf("len = %d, want 3, got %v", len(result), result) + } +} + +func TestSplitKeywords_FiltersEmptyStrings(t *testing.T) { + result := SplitKeywords("kw1,,kw2") + if len(result) != 2 { + t.Errorf("empty strings should be filtered: got %v", result) + } +} + +func TestSplitKeywords_Empty(t *testing.T) { + result := SplitKeywords("") + if len(result) != 0 { + t.Errorf("got %v, want empty", result) + } +} + +// SplitQuestions - splits by newline only, dropping empty lines. + +func TestSplitQuestions_MultipleLines(t *testing.T) { + result := SplitQuestions("Q1\nQ2\nQ3") + if len(result) != 3 || result[0] != "Q1" || result[2] != "Q3" { + t.Fatalf("got %v, want [Q1 Q2 Q3]", result) + } +} + +func TestSplitQuestions_Empty(t *testing.T) { + if result := SplitQuestions(""); result != nil { + t.Errorf("got %v, want nil", result) + } +} + +func TestSplitQuestions_TrailingNewlineDropped(t *testing.T) { + // "Q1\n" would yield ["Q1", ""] with a plain split; the trailing empty + // must be filtered so the index is not seeded with a spurious [""]. + result := SplitQuestions("Q1\n") + if len(result) != 1 || result[0] != "Q1" { + t.Fatalf("got %v, want [Q1]", result) + } +} + +func TestSplitQuestions_BlankLinesDropped(t *testing.T) { + result := SplitQuestions("Q1\n\nQ2") + if len(result) != 2 || result[0] != "Q1" || result[1] != "Q2" { + t.Fatalf("got %v, want [Q1 Q2]", result) + } +} + +func TestSplitQuestions_PreservesCommas(t *testing.T) { + // Unlike SplitKeywords, questions must NOT split on commas. + result := SplitQuestions("请问A,B和C有何区别？") + if len(result) != 1 || result[0] != "请问A,B和C有何区别？" { + t.Fatalf("got %v, want single element preserving comma", result) + } +} diff --git a/test/testcases/restful_api/test_datasets.py b/test/testcases/restful_api/test_datasets.py index caa763286a..cb746984d1 100644 --- a/test/testcases/restful_api/test_datasets.py +++ b/test/testcases/restful_api/test_datasets.py @@ -902,7 +902,7 @@ def test_dataset_update_chunk_method_invalid_contract(rest_client, clear_dataset if IS_GO_PROXY and not isinstance(chunk_method, str): assert "cannot unmarshal" in payload["message"] and ".chunk_method" in payload["message"], payload elif IS_GO_PROXY: - assert payload["message"].startswith("Input should be 'naive', 'book'") and payload["message"].endswith("or 'tag'"), payload + assert payload["message"].startswith("Input should be 'audio', 'book'") and payload["message"].endswith("or 'tag'"), payload else: assert expected_chunk_message in payload["message"], payload @@ -912,7 +912,7 @@ def test_dataset_update_chunk_method_invalid_contract(rest_client, clear_dataset _skip_go_ignored_null(none_payload, "chunk_method") assert none_payload["code"] == ARGUMENT_ERROR_CODE, none_payload if IS_GO_PROXY: - assert none_payload["message"].startswith("Input should be 'naive', 'book'") and none_payload["message"].endswith("or 'tag'"), none_payload + assert none_payload["message"].startswith("Input should be 'audio', 'book'") and none_payload["message"].endswith("or 'tag'"), none_payload else: assert expected_chunk_message in none_payload["message"], none_payload @@ -1664,7 +1664,7 @@ def test_dataset_create_permission_and_chunk_method_contract(rest_client, clear_ if IS_GO_PROXY and not isinstance(chunk_method, str): assert "cannot unmarshal" in payload["message"] and ".chunk_method" in payload["message"], payload elif IS_GO_PROXY: - assert payload["message"].startswith("Input should be 'naive', 'book'") and payload["message"].endswith("or 'tag'"), payload + assert payload["message"].startswith("Input should be 'audio', 'book'") and payload["message"].endswith("or 'tag'"), payload else: assert expected_chunk_message in payload["message"], payload @@ -1673,7 +1673,7 @@ def test_dataset_create_permission_and_chunk_method_contract(rest_client, clear_ chunk_method_none_payload = chunk_method_none_res.json() assert chunk_method_none_payload["code"] == ARGUMENT_ERROR_CODE, chunk_method_none_payload if IS_GO_PROXY: - assert chunk_method_none_payload["message"].startswith("Input should be 'naive', 'book'") and chunk_method_none_payload["message"].endswith("or 'tag'"), chunk_method_none_payload + assert chunk_method_none_payload["message"].startswith("Input should be 'audio', 'book'") and chunk_method_none_payload["message"].endswith("or 'tag'"), chunk_method_none_payload else: assert expected_chunk_message in chunk_method_none_payload["message"], chunk_method_none_payload diff --git a/web/src/components/chunk-method-dialog/hooks.ts b/web/src/components/chunk-method-dialog/hooks.ts index 0dd5a7fee3..ef24872f2a 100644 --- a/web/src/components/chunk-method-dialog/hooks.ts +++ b/web/src/components/chunk-method-dialog/hooks.ts @@ -1,108 +1,4 @@ -import { useSelectParserList } from '@/hooks/use-user-setting-request'; -import { useCallback, useMemo } from 'react'; - -const ParserListMap = new Map([ - [ - ['pdf'], - [ - 'naive', - 'resume', - 'manual', - 'paper', - 'book', - 'laws', - 'presentation', - 'one', - 'qa', - 'knowledge_graph', - ], - ], - [ - ['doc', 'docx'], - [ - 'naive', - 'resume', - 'book', - 'laws', - 'one', - 'qa', - 'manual', - 'knowledge_graph', - ], - ], - [ - ['xlsx', 'xls'], - ['naive', 'qa', 'table', 'one', 'knowledge_graph'], - ], - [['ppt', 'pptx'], ['presentation']], - [ - ['jpg', 'jpeg', 'png', 'gif', 'bmp', 'tif', 'tiff', 'webp', 'svg', 'ico'], - ['picture'], - ], - [ - ['txt'], - [ - 'naive', - 'resume', - 'book', - 'laws', - 'one', - 'qa', - 'table', - 'knowledge_graph', - ], - ], - [ - ['csv'], - [ - 'naive', - 'resume', - 'book', - 'laws', - 'one', - 'qa', - 'table', - 'knowledge_graph', - ], - ], - [ - ['md', 'mdx'], - ['naive', 'qa', 'knowledge_graph'], - ], - [['json'], ['naive', 'knowledge_graph']], - [['eml'], ['email']], -]); - -const getParserList = ( - values: string[], - parserList: Array<{ - value: string; - label: string; - }>, -) => { - return parserList.filter((x) => values?.some((y) => y === x.value)); -}; - -export const useFetchParserListOnMount = (documentExtension: string) => { - const parserList = useSelectParserList(); - - const nextParserList = useMemo(() => { - const key = [...ParserListMap.keys()].find((x) => - x.some((y) => y === documentExtension), - ); - if (key) { - const values = ParserListMap.get(key); - return getParserList(values ?? [], parserList); - } - - return getParserList( - ['naive', 'resume', 'book', 'laws', 'one', 'qa', 'table'], - parserList, - ); - }, [parserList, documentExtension]); - - return { parserList: nextParserList }; -}; +import { useCallback } from 'react'; const hideAutoKeywords = ['qa', 'table', 'resume', 'knowledge_graph', 'tag']; diff --git a/web/src/hooks/use-user-setting-request.tsx b/web/src/hooks/use-user-setting-request.tsx index 3b498e2cff..41a487e225 100644 --- a/web/src/hooks/use-user-setting-request.tsx +++ b/web/src/hooks/use-user-setting-request.tsx @@ -10,6 +10,7 @@ import { } from '@/interfaces/database/user-setting'; import { ISetLangfuseConfigRequestBody } from '@/interfaces/request/system'; import { DEFAULT_LANGUAGE_CODE, supportedLanguages } from '@/locales/config'; +import kbService from '@/services/knowledge-service'; import userService, { addTenantUser, agreeTenant, @@ -102,51 +103,46 @@ export const useSelectParserList = (): Array<{ const { data: tenantInfo } = useFetchTenantInfo(true); const { t } = useTranslation(); - const defaultParsers = useMemo( - () => [ - { value: 'naive', label: t('knowledgeConfiguration.parserLabel.naive') }, - { value: 'qa', label: t('knowledgeConfiguration.parserLabel.qa') }, - { - value: 'resume', - label: t('knowledgeConfiguration.parserLabel.resume'), - }, - { - value: 'manual', - label: t('knowledgeConfiguration.parserLabel.manual'), - }, - { value: 'table', label: t('knowledgeConfiguration.parserLabel.table') }, - { value: 'paper', label: t('knowledgeConfiguration.parserLabel.paper') }, - { value: 'book', label: t('knowledgeConfiguration.parserLabel.book') }, - { value: 'laws', label: t('knowledgeConfiguration.parserLabel.laws') }, - { - value: 'presentation', - label: t('knowledgeConfiguration.parserLabel.presentation'), - }, - { - value: 'picture', - label: t('knowledgeConfiguration.parserLabel.picture'), - }, - { value: 'one', label: t('knowledgeConfiguration.parserLabel.one') }, - { value: 'audio', label: t('knowledgeConfiguration.parserLabel.audio') }, - { value: 'email', label: t('knowledgeConfiguration.parserLabel.email') }, - { value: 'tag', label: t('knowledgeConfiguration.parserLabel.tag') }, - ], - [t], - ); + // Fetch the builtin pipeline catalog from the backend (embed-registry), + // replacing the old hardcoded parser list. + const { data: pipelineListData } = useQuery({ + queryKey: ['listPipelines'], + queryFn: async () => { + const { data } = await kbService.listPipelines(); + return data; + }, + staleTime: Infinity, + }); + const pipelineList: Array<{ + parser_id: string; + title: string; + dsl: Record; + }> = pipelineListData?.data ?? []; const parserList = useMemo(() => { const parserArray: Array = tenantInfo?.parser_ids?.split(',') ?? []; const filteredArray = parserArray.filter((x) => x.trim() !== ''); - if (filteredArray.length === 0) { - return defaultParsers; + // Tenant-level parser_ids filter (if configured), otherwise all builtins. + if (filteredArray.length > 0) { + return filteredArray.map((x) => { + const arr = x.split(':'); + return { value: arr[0], label: arr[1] }; + }); } - return filteredArray.map((x) => { - const arr = x.split(':'); - return { value: arr[0], label: arr[1] }; - }); - }, [tenantInfo, defaultParsers]); + // Map API pipeline list to { value, label }; prefer i18n label, + // fallback to the API title when the i18n key is missing. + const labelFromAPI = (parserId: string, title: string) => { + const key = `knowledgeConfiguration.parserLabel.${parserId}`; + const translated = t(key); + return translated !== key ? translated : title; + }; + return pipelineList.map((item) => ({ + value: item.parser_id, + label: labelFromAPI(item.parser_id, item.title), + })); + }, [tenantInfo, pipelineList, t]); return parserList; }; diff --git a/web/src/locales/en.ts b/web/src/locales/en.ts index f14e8077d7..b4ee9a5056 100644 --- a/web/src/locales/en.ts +++ b/web/src/locales/en.ts @@ -778,6 +778,7 @@ Example: A 1 KB message with 1024-dim embedding uses ~9 KB. The 5 MB default lim 'Re-parse existing documents for the new column roles to take effect.', parserLabel: { naive: 'General', + general: 'General', qa: 'Q&A', resume: 'Resume', manual: 'Manual', diff --git a/web/src/services/knowledge-service.ts b/web/src/services/knowledge-service.ts index bebfffeb31..3b21f8790b 100644 --- a/web/src/services/knowledge-service.ts +++ b/web/src/services/knowledge-service.ts @@ -19,6 +19,7 @@ const { documentThumbnails, documentIngest, listTagByKnowledgeIds, + listPipelines, setMeta, getMeta, getMetaKeys, @@ -66,6 +67,10 @@ const methods = { url: retrievalTestShare, method: 'post', }, + listPipelines: { + url: listPipelines, + method: 'get', + }, pipelineRerun: { url: api.pipelineRerun, method: 'post', diff --git a/web/src/utils/api.ts b/web/src/utils/api.ts index fa212a24a1..892c119df6 100644 --- a/web/src/utils/api.ts +++ b/web/src/utils/api.ts @@ -181,6 +181,7 @@ export default { `${restAPIv1}/datasets/${datasetId}/ingestions/${logId}`, fetchPipelineDatasetLogs: (datasetId: string) => `${restAPIv1}/datasets/${datasetId}/ingestions`, + listPipelines: `${restAPIv1}/pipelines?type=builtin`, runIndex: (datasetId: string, indexType: string) => `${restAPIv1}/datasets/${datasetId}/index?type=${indexType.toLowerCase()}`, traceIndex: (datasetId: string, indexType: string) =>