Go ports of workflow and chunker fixes (#16878)

Ports two Python fixes to Go: the variable_ref_patt underscore/colon fix
(#16792) and the TokenChunker upstream-chunks fix (#16825). Keeps Go
behavior aligned with upstream Python.
This commit is contained in:
Zhichang Yu
2026-07-13 23:32:07 +08:00
committed by GitHub
parent e54e7ec7ef
commit d279aee1ff
4 changed files with 208 additions and 14 deletions

View File

@@ -14,15 +14,19 @@
// in canvas.get_value_with_variable AFTER the regex match succeeds.
// - list indexing (xs.0) — same nested-path machinery.
//
// Cpn IDs in tests use underscores (e.g. "llm_0") which is the real
// RAGFlow naming convention; the original documented regex
// `[a-zA-Z:0-9]+` did not allow underscores — see variable.go
// VarRefPattern comment.
// The VarRefPattern regex (internal/agent/runtime/template.go) is kept
// in sync with Python's agent/component/base.py variable_ref_patt.
// PR #16792 (Jun 2026) widened cpn_id from [a-zA-Z:0-9]+ to
// [a-zA-Z0-9_:]+ to accept underscored frontend ids and colon-bearing
// legacy DSL ids. This file pins both behaviors to prevent silent
// regression.
package canvas
import (
"reflect"
"testing"
"ragflow/internal/agent/runtime"
)
func TestVariableResolver(t *testing.T) {
@@ -203,3 +207,144 @@ func TestExtractRefs(t *testing.T) {
t.Fatalf("ExtractRefs: got %v want %v", got, want)
}
}
// TestVarRefPattern_MatchesUnderscoredComponentIDs pins that frontend-
// emitted ids like "userfillup_abc@line" match VarRefPattern.
// Regression test for the original #16758 underscore fix
// (Python equivalent: test_variable_ref_patt_matches_underscored_component_ids).
func TestVarRefPattern_MatchesUnderscoredComponentIDs(t *testing.T) {
cases := []struct {
text string
expected string // group-1 capture (bare ref)
}{
{"{userfillup_abc@line}", "userfillup_abc@line"},
{"{retrieval_xyz@chunks}", "retrieval_xyz@chunks"},
{"{llm_0@content}", "llm_0@content"},
{"{message_0@answer}", "message_0@answer"},
}
for _, c := range cases {
t.Run(c.text, func(t *testing.T) {
matches := VarRefPattern.FindAllStringSubmatch(c.text, 1)
if len(matches) == 0 || len(matches[0]) < 2 {
t.Fatalf("Expected %q to match VarRefPattern", c.text)
}
got := matches[0][1]
if got != c.expected {
t.Fatalf("%q: wrong capture — got %q, expected %q", c.text, got, c.expected)
}
})
}
}
// TestVarRefPattern_MatchesColonBearingComponentIDs pins that legacy
// DSL ids like "UserFillUp:CateInput@text" match VarRefPattern.
// Regression test from PR #16792 review note — colon-bearing cpn_ids
// are real and used in test fixtures/templates
// (Python equivalent: test_variable_ref_patt_matches_colon_bearing_component_ids).
func TestVarRefPattern_MatchesColonBearingComponentIDs(t *testing.T) {
cases := []struct {
text string
expected string
}{
{"{UserFillUp:CateInput@text}", "UserFillUp:CateInput@text"},
{"{UserFillUp:CodeInput@x}", "UserFillUp:CodeInput@x"},
{"{UserFillUp:LoopInput@value}", "UserFillUp:LoopInput@value"},
{"{Retrieval:KBSearch@formalized_content}", "Retrieval:KBSearch@formalized_content"},
{"{CodeExec:Double@result}", "CodeExec:Double@result"},
{"{Browser:BusyHatsSink@content}", "Browser:BusyHatsSink@content"},
}
for _, c := range cases {
t.Run(c.text, func(t *testing.T) {
matches := VarRefPattern.FindAllStringSubmatch(c.text, 1)
if len(matches) == 0 || len(matches[0]) < 2 {
t.Fatalf("Expected %q to match VarRefPattern — colon-bearing cpn_id lost its support.", c.text)
}
got := matches[0][1]
if got != c.expected {
t.Fatalf("%q: wrong capture — got %q, expected %q", c.text, got, c.expected)
}
})
}
}
// TestVarRefPattern_StillMatchesLegacyIDs verifies backward-compat:
// legacy ids without underscores/colons must still resolve.
func TestVarRefPattern_StillMatchesLegacyIDs(t *testing.T) {
cases := []struct {
text string
expected string
}{
{"{begin@line}", "begin@line"},
{"{retrieval@chunks}", "retrieval@chunks"},
{"{sys.query}", "sys.query"},
{"{sys.user_id}", "sys.user_id"},
{"{env.HOME}", "env.HOME"},
}
for _, c := range cases {
t.Run(c.text, func(t *testing.T) {
matches := VarRefPattern.FindAllStringSubmatch(c.text, 1)
if len(matches) == 0 || len(matches[0]) < 2 {
t.Fatalf("Expected %q to match VarRefPattern", c.text)
}
if matches[0][1] != c.expected {
t.Fatalf("%q: wrong capture — got %q, expected %q", c.text, matches[0][1], c.expected)
}
})
}
}
// TestVarRefPattern_DoesNotMatchBareVarName verifies that
// `{line}` without a cpn_id prefix is intentionally not a template
// ref — it must remain literal so the user sees the literal text.
func TestVarRefPattern_DoesNotMatchBareVarName(t *testing.T) {
if VarRefPattern.MatchString("{line}") {
t.Fatal("Bare `{line}` should not match — only `cpn_id@var` / `sys.*` / `env.*` are valid template refs.")
}
}
// TestVarRefPattern_ConsistencyPin verifies that the runtime.VarRefPattern
// pattern string and its factory-compiled regex agree. This guards against
// accidental edits that change the string without updating the compile.
func TestVarRefPattern_ConsistencyPin(t *testing.T) {
// The Go regexp package does not expose a way to reconstruct a
// regexp from its string representation at test time while
// preserving the same object identity, so we instead pin that the
// source string matches the expected pattern character-by-character.
want := `\{+\s*([a-zA-Z:0-9_]+@[A-Za-z0-9_.-]+|sys\.[A-Za-z0-9_.]+|env\.[A-Za-z0-9_.]+|item|index)\s*\}+`
if runtime.VarRefPattern.String() != want {
t.Errorf(
"VarRefPattern string has drifted.\n got %s\n want %s\n"+
"Did you edit the regex? If so, update this test AND the Python source at agent/component/base.py.",
runtime.VarRefPattern.String(), want,
)
}
}
// TestResolveTemplate_SubstitutesUnderscoredRef ensures that
// an underscored ref is substituted end-to-end through ResolveTemplate.
func TestResolveTemplate_SubstitutesUnderscoredRef(t *testing.T) {
s := NewCanvasState("run-1", "task-1")
s.SetVar("userfillup_abc", "line", "hello world")
rendered, err := ResolveTemplate("Repeat: {{userfillup_abc@line}}", s)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if rendered != "Repeat: hello world" {
t.Fatalf("got %q want %q", rendered, "Repeat: hello world")
}
}
// TestExtractRefs_ColonBearing verifies ExtractRefs returns colon-bearing refs.
func TestExtractRefs_ColonBearing(t *testing.T) {
got := ExtractRefs(
"{{UserFillUp:CateInput@text}} {{Retrieval:KBSearch@f}} {{sys.query}}",
)
want := []string{"UserFillUp:CateInput@text", "Retrieval:KBSearch@f", "sys.query"}
if !reflect.DeepEqual(got, want) {
t.Fatalf("ExtractRefs colon-bearing: got %v want %v", got, want)
}
}

View File

@@ -28,20 +28,16 @@ import (
)
// VarRefPattern matches the RAGFlow variable reference syntax.
// Mirrors agent/component/base.py:368 in spirit with one deviation: the
// cpn_id part includes '_' (real RAGFlow cpn_ids are like "begin_0",
// "llm_0", "cpn_0"). The Python regex as documented in the plan
// (`[a-zA-Z:0-9]+`) would not match those — this looks like a
// documentation bug in the plan; the Python source likely has
// the underscore too. The pattern uses underscore-friendly
// matching; a future cross-check against the live Python source
// can confirm the exact behavior.
// Matches agent/component/base.py variable_ref_patt exactly after
// PR #16792 (which widened cpn_id from [a-zA-Z:0-9]+ to
// [a-zA-Z0-9_:]+ to accept underscores in frontend-emitted ids as
// well as colons in legacy DSL ids).
//
// Pattern:
//
// \{+\s*(<ref>)\s*\}+
// where <ref> = cpn_id@param | sys.x | env.x | item | index
// cpn_id = [a-zA-Z:0-9_]+ (note: underscore added; see deviation note)
// cpn_id = [a-zA-Z:0-9_]+ (character classes match Python's [a-zA-Z0-9_:]+)
// param = [A-Za-z0-9_.-]+
//
// Capture group 1 holds the bare ref without braces (e.g. "cpn_0@content",

View File

@@ -193,6 +193,20 @@ func (c *TokenChunkerComponent) invoke(ctx context.Context, inputs map[string]an
}
return c.invokeTextPayload(ctx, *upstream.HTMLResult, delimPattern, childrenPattern), nil
default:
// Port of token_chunker.py:347 — when the upstream emitted
// chunks (output_format == "chunks", e.g. a TitleChunker
// feeding into this TokenChunker), consume those chunks rather
// than the raw parser json_result. Otherwise fall back to the
// structured json_result. This fixes #16812 where a
// TitleChunker → TokenChunker chain silently discarded the
// chapter-level chunks and re-chunked the raw parser output.
var items []schema.ChunkDoc
if upstream.OutputFormat == schema.PayloadFormatChunks {
items = upstream.Chunks
} else {
items = upstream.JSONResult
}
// Re-acquire the source PDF (if the Parser forwarded storage
// refs) so image/table sections are cropped on demand rather
// than carried through the wire. Best-effort: a nil engine
@@ -204,7 +218,7 @@ func (c *TokenChunkerComponent) invoke(ctx context.Context, inputs map[string]an
if engine != nil {
defer engine.Close()
}
return c.invokeJSONPayload(ctx, upstream.JSONResult, delimPattern, childrenPattern, engine), nil
return c.invokeJSONPayload(ctx, items, delimPattern, childrenPattern, engine), nil
}
}

View File

@@ -284,3 +284,42 @@ func TestTokenChunker_NewAcceptsDefaults(t *testing.T) {
t.Errorf("default delimiter_mode = %q, want token_size", got)
}
}
// TestTokenChunker_PrefersUpstreamChunks is the Go port of the Python
// regression test for #16812 (PR #16825). When a TitleChunker feeds
// this TokenChunker with output_format == "chunks" AND both a "chunks"
// list and a raw "json" list on the wire, the TokenChunker must
// consume the upstream chunks (CHAPTER-AWARE) and must NOT fall through
// to the raw parser json_result (RAW-PARSER-JSON).
func TestTokenChunker_PrefersUpstreamChunks(t *testing.T) {
c, err := NewTokenChunker(map[string]any{
"delimiter_mode": "delimiter",
"delimiters": []string{"\n"},
})
if err != nil {
t.Fatalf("NewTokenChunker: %v", err)
}
out, err := c.Invoke(context.Background(), map[string]any{
"name": "doc.md",
"output_format": "chunks",
"chunks": []map[string]any{{"text": "CHAPTER-AWARE", "doc_type_kwd": "text"}},
"json": []map[string]any{{"text": "RAW-PARSER-JSON", "doc_type_kwd": "text"}},
})
if err != nil {
t.Fatalf("Invoke: %v", err)
}
chunks, _ := out["chunks"].([]map[string]any)
if len(chunks) == 0 {
t.Fatal("chunks: want >=1, got 0")
}
for i, ck := range chunks {
text, _ := ck["text"].(string)
if text == "RAW-PARSER-JSON" {
t.Fatalf("chunk[%d] consumed the raw parser json_result instead of upstream chunks: %q", i, text)
}
if text == "CHAPTER-AWARE" {
return // happy path: upstream chunk preserved
}
}
t.Fatalf("upstream chunk 'CHAPTER-AWARE' was not found in output: %v", out["chunks"])
}