fix(ingestion): derive component param whitelist from component schemas (#20060)

internal/ingestion/pipeline/pipeline_params.go

@@ -40,6 +40,27 @@ var extractorValidParamKeys = func() map[string]struct{} {
 	return keys
 }()
 
+// componentParamSchemaKeys maps a DSL component_name (lower-cased) to the
+// param keys its param schema declares. CleanComponentParams unions these with
+// the keys a template already bakes, so a param the component can read is
+// never dropped just because a template — or a canvas snapshot saved before
+// the param existed — does not carry it.
+//
+// Components whose accepted keys are defined by the DSL itself (Parser file
+// families) or whose param struct is not part of the schema package
+// (GeneralChunker, QAChunker) stay out of this table; Extractor and Compiler
+// keep their cpnID-prefixed dynamic whitelists.
+var componentParamSchemaKeys = map[string]map[string]struct{}{
+	"titlechunker": extractJSONTags(schema.TitleChunkerParam{}),
+	// ManualChunker reuses TitleChunkerParam. It pins method=group and is exempt
+	// from the token cap, so method, chunk_token_cap and include_heading_content
+	// (hierarchy-only) are accepted here but ignored by the component — the
+	// operator form omits them for the same reason.
+	"manualchunker": extractJSONTags(schema.TitleChunkerParam{}),
+	"tokenchunker":  extractJSONTags(schema.TokenChunkerParam{}),
+	"tokenizer":     extractJSONTags(schema.TokenizerParam{}),
+}
+
 // extractJSONTags returns all top-level json tag names from a struct.
 func extractJSONTags(v any) map[string]struct{} {
 	tags := make(map[string]struct{})
@@ -98,9 +119,11 @@ func getComponentParamWhitelist(cpnID string) (map[string]struct{}, bool) {
 
 // CleanComponentParams filters rawConfig against the DSL schema given by dslJSON.
 // Keys containing ':' are treated as component IDs; they are kept only when both
-// the cpnID AND the param name exist in the DSL schema or the component's dynamic
-// parameter schema (e.g. Extractor modular features). Keys without ':' (legacy
-// flat fields) are dropped with a warning.
+// the cpnID AND the param name exist in one of the accepted-key sources: the DSL
+// schema (the params the template bakes), the schema-derived per-component table
+// (componentParamSchemaKeys), or the component's dynamic parameter schema (e.g.
+// Extractor modular features). Keys without ':' (legacy flat fields) are dropped
+// with a warning.
 func CleanComponentParams(dslJSON []byte, rawConfig map[string]interface{}) map[string]interface{} {
 	schemas, err := ExtractAllComponentParams(dslJSON)
 	if err != nil {
@@ -122,6 +145,9 @@ func CleanComponentParams(dslJSON []byte, rawConfig map[string]interface{}) map[
 		if IsChunkerComponent(s.CpnID) {
 			keys["enable_children"] = struct{}{}
 		}
+		for k := range componentParamSchemaKeys[strings.ToLower(s.ComponentName)] {
+			keys[k] = struct{}{}
+		}
 		validCPNs[s.CpnID] = keys
 		componentNames[s.CpnID] = s.ComponentName
 	}

internal/ingestion/pipeline/pipeline_params_test.go

@@ -3,6 +3,7 @@ package pipeline
 import (
 	"encoding/json"
 	"reflect"
+	"strings"
 	"testing"
 
 	"ragflow/internal/entity"
@@ -791,3 +792,256 @@ func TestBuildParserConfig_CompilerRuntimeParamsSurvive(t *testing.T) {
 		t.Errorf("expected DSL default plan=false to be baked in, got %#v", params["plan"])
 	}
 }
+
+// --- componentParamSchemaKeys (schema-derived component param whitelist) ---
+
+// titleFamilyDSL mirrors a canvas snapshot saved before chunk_token_cap /
+// root_chunk_as_heading existed: the title-chunker node bakes only the older
+// params.
+func titleFamilyDSL(t *testing.T) []byte {
+	t.Helper()
+	dsl := map[string]any{
+		"components": map[string]any{
+			"TitleChunker:StaleSnapshot": map[string]any{
+				"obj": map[string]any{
+					"component_name": "TitleChunker",
+					"params": map[string]any{
+						"outputs":   map[string]any{},
+						"method":    "hierarchy",
+						"hierarchy": float64(3),
+						"levels":    []any{[]any{"^#[^#]"}},
+					},
+				},
+			},
+		},
+	}
+	raw, err := json.Marshal(dsl)
+	if err != nil {
+		t.Fatalf("marshal dsl fixture: %v", err)
+	}
+	return raw
+}
+
+func TestCleanComponentParams_TitleFamilyKeepsSchemaParamsWithoutBakedKeys(t *testing.T) {
+	dslJSON := titleFamilyDSL(t)
+	raw := map[string]any{
+		"TitleChunker:StaleSnapshot": map[string]any{
+			"chunk_token_cap":       256,
+			"root_chunk_as_heading": true,
+		},
+	}
+	result := CleanComponentParams(dslJSON, raw)
+	params, ok := result["TitleChunker:StaleSnapshot"].(map[string]any)
+	if !ok {
+		t.Fatalf("expected TitleChunker params to survive, got %#v", result)
+	}
+	if _, ok := params["chunk_token_cap"]; !ok {
+		t.Errorf("chunk_token_cap dropped although the component schema declares it: %#v", params)
+	}
+	if _, ok := params["root_chunk_as_heading"]; !ok {
+		t.Errorf("root_chunk_as_heading dropped although the component schema declares it: %#v", params)
+	}
+}
+
+// TestBuildParserConfig_TitleFamilyParamsSurviveBuiltinTemplates locks the
+// dataset/document save path for the title-chunker family: both params must
+// survive BuildParserConfig for every builtin template that carries a
+// TitleChunker or ManualChunker node, none of which bake the two keys in their
+// DSL params.
+func TestBuildParserConfig_TitleFamilyParamsSurviveBuiltinTemplates(t *testing.T) {
+	registry, err := DefaultRegistry()
+	if err != nil {
+		t.Fatalf("DefaultRegistry: %v", err)
+	}
+	tested := 0
+	for _, ref := range registry.Refs() {
+		tpl, ok := registry.Get(ref)
+		if !ok {
+			continue
+		}
+		dslJSON, err := json.Marshal(tpl.DSL)
+		if err != nil {
+			t.Fatalf("marshal DSL %q: %v", ref, err)
+		}
+		schemas, err := ExtractAllComponentParams(dslJSON)
+		if err != nil {
+			t.Fatalf("ExtractAllComponentParams %q: %v", ref, err)
+		}
+		for _, s := range schemas {
+			if s.ComponentName != "TitleChunker" && s.ComponentName != "ManualChunker" {
+				continue
+			}
+			tested++
+			overrides := map[string]interface{}{
+				s.CpnID: map[string]any{
+					"chunk_token_cap":       256,
+					"root_chunk_as_heading": true,
+				},
+			}
+			params, ok := BuildParserConfig(dslJSON, overrides)[s.CpnID].(map[string]any)
+			if !ok {
+				t.Fatalf("template %q: component %q missing from built parser_config", ref, s.CpnID)
+			}
+			if !reflect.DeepEqual(params["chunk_token_cap"], 256) {
+				t.Errorf("template %q %s: chunk_token_cap = %#v, want 256", ref, s.CpnID, params["chunk_token_cap"])
+			}
+			if !reflect.DeepEqual(params["root_chunk_as_heading"], true) {
+				t.Errorf("template %q %s: root_chunk_as_heading = %#v, want true", ref, s.CpnID, params["root_chunk_as_heading"])
+			}
+		}
+	}
+	if tested == 0 {
+		t.Fatal("expected at least one builtin template with a TitleChunker/ManualChunker component")
+	}
+}
+
+// operatorFormParamKeys mirrors the params the dataset/document pipeline
+// operator forms submit (transformFormConfigToApi in
+// web/src/utils/pipeline-operator.ts). Every key listed for a component must be
+// accepted by CleanComponentParams for that component in every builtin
+// template; otherwise a user edit is silently dropped.
+var operatorFormParamKeys = map[string][]string{
+	"TitleChunker": {
+		"method", "levels", "hierarchy", "include_heading_content",
+		"root_chunk_as_heading", "chunk_token_cap",
+	},
+	"ManualChunker": {"method", "levels", "hierarchy", "root_chunk_as_heading"},
+	"TokenChunker": {
+		"chunk_token_size", "overlapped_percent", "delimiters", "delimiter_mode",
+		"children_delimiters", "enable_children", "table_context_size", "image_context_size",
+	},
+	"GeneralChunker": {
+		"chunk_token_size", "overlapped_percent", "delimiters", "children_delimiters",
+		"enable_children", "table_context_size", "image_context_size",
+	},
+	"Tokenizer": {"search_method", "filename_embd_weight", "fields"},
+	"Extractor": {"llm_id", "keywords", "questions", "tags", "summary", "metadata"},
+}
+
+func TestCleanComponentParams_AcceptsOperatorFormParams(t *testing.T) {
+	registry, err := DefaultRegistry()
+	if err != nil {
+		t.Fatalf("DefaultRegistry: %v", err)
+	}
+	checked := 0
+	for _, ref := range registry.Refs() {
+		tpl, ok := registry.Get(ref)
+		if !ok {
+			continue
+		}
+		dslJSON, err := json.Marshal(tpl.DSL)
+		if err != nil {
+			t.Fatalf("marshal DSL %q: %v", ref, err)
+		}
+		schemas, err := ExtractAllComponentParams(dslJSON)
+		if err != nil {
+			t.Fatalf("ExtractAllComponentParams %q: %v", ref, err)
+		}
+		for _, s := range schemas {
+			expected, ok := operatorFormParamKeys[s.ComponentName]
+			if !ok {
+				continue
+			}
+			checked++
+			params := make(map[string]any, len(expected))
+			for _, key := range expected {
+				params[key] = "probe"
+			}
+			cleaned := CleanComponentParams(dslJSON, map[string]interface{}{s.CpnID: params})
+			got, ok := cleaned[s.CpnID].(map[string]any)
+			if !ok {
+				t.Fatalf("template %q: component %q missing from cleaned result", ref, s.CpnID)
+			}
+			for _, key := range expected {
+				if _, ok := got[key]; !ok {
+					t.Errorf("template %q component %s: operator-form param %q is silently dropped", ref, s.CpnID, key)
+				}
+			}
+		}
+	}
+	if checked == 0 {
+		t.Fatal("expected at least one builtin template component matched by operatorFormParamKeys")
+	}
+}
+
+// componentParamWhitelistExemptions lists the builtin template component names
+// that intentionally have no entry in componentParamSchemaKeys, with the reason.
+// The inventory test below fails on any component that is neither derived nor
+// exempt, so a renamed or newly added component cannot silently lose its
+// whitelist coverage.
+var componentParamWhitelistExemptions = map[string]string{
+	"File":           "no user-editable params",
+	"Parser":         "accepted keys are the DSL's file families",
+	"GeneralChunker": "param struct lives in the chunker package without json tags; template keys + the delimiters exception cover it",
+	"QAChunker":      "no operator form; param struct is not in the schema package",
+	"OneChunker":     "no user-editable params",
+	"PageChunker":    "no user-editable params",
+	"TableChunker":   "no user-editable params",
+	"Extractor":      "covered by the cpnID-prefixed dynamic whitelist",
+	"Compiler":       "covered by the cpnID-prefixed dynamic whitelist",
+}
+
+// TestBuiltinTemplateComponentsHaveWhitelistCoverage pins the component-name
+// inventory of the builtin templates: every name must either derive its param
+// whitelist from a schema (componentParamSchemaKeys) or be exempted above. It
+// guards the operator-form test against becoming a no-op: that test matches by
+// component name, so a rename would otherwise silently skip it.
+func TestBuiltinTemplateComponentsHaveWhitelistCoverage(t *testing.T) {
+	registry, err := DefaultRegistry()
+	if err != nil {
+		t.Fatalf("DefaultRegistry: %v", err)
+	}
+	seen := make(map[string]string)
+	for _, ref := range registry.Refs() {
+		tpl, ok := registry.Get(ref)
+		if !ok {
+			continue
+		}
+		dslJSON, err := json.Marshal(tpl.DSL)
+		if err != nil {
+			t.Fatalf("marshal DSL %q: %v", ref, err)
+		}
+		schemas, err := ExtractAllComponentParams(dslJSON)
+		if err != nil {
+			t.Fatalf("ExtractAllComponentParams %q: %v", ref, err)
+		}
+		for _, s := range schemas {
+			seen[s.ComponentName] = ref
+		}
+	}
+	if len(seen) == 0 {
+		t.Fatal("expected builtin templates to contain components")
+	}
+	for name, ref := range seen {
+		if _, ok := componentParamSchemaKeys[strings.ToLower(name)]; ok {
+			continue
+		}
+		if _, ok := componentParamWhitelistExemptions[name]; ok {
+			continue
+		}
+		t.Errorf("component %q (template %q) is neither schema-derived nor exempt: add it to componentParamSchemaKeys or exempt it with a reason", name, ref)
+	}
+}
+
+// TestCleanComponentParams_TitleFamilyStillDropsUnknownKeys keeps the union
+// honest: a key the component schema does not declare is still dropped.
+func TestCleanComponentParams_TitleFamilyStillDropsUnknownKeys(t *testing.T) {
+	dslJSON := titleFamilyDSL(t)
+	raw := map[string]any{
+		"TitleChunker:StaleSnapshot": map[string]any{
+			"chunk_token_cap_typo": 256,
+			"method":               "hierarchy",
+		},
+	}
+	result := CleanComponentParams(dslJSON, raw)
+	params, ok := result["TitleChunker:StaleSnapshot"].(map[string]any)
+	if !ok {
+		t.Fatalf("expected TitleChunker params to survive, got %#v", result)
+	}
+	if _, ok := params["chunk_token_cap_typo"]; ok {
+		t.Errorf("unknown key must still be dropped, got %#v", params)
+	}
+	if _, ok := params["method"]; !ok {
+		t.Errorf("declared key must survive, got %#v", params)
+	}
+}