diff --git a/docs/flows.md b/docs/flows.md index 36a1ebba9a..7f8dcab977 100644 --- a/docs/flows.md +++ b/docs/flows.md @@ -652,6 +652,8 @@ Prompts support `${args.*}` and `${step.*}` placeholders: Dot-paths walk nested `map[string]any` values one segment at a time. If any segment is missing or points to a non-map value, the literal placeholder is preserved so downstream tooling (e.g. `flow.session.prefix` validation) can flag the miss. Top-level exact-key match wins first, so a flat key that literally contains a dot (e.g. `"a.b"` in args) resolves before the resolver falls back to walking `a["b"]`. Predicates use the same resolver, so `${args.reviewer.email} == user@x.com` works in `if:` rules. +Scalar values substitute as plain text (a string verbatim, so quoting in the prompt like `aid: "${args.aid}"` stays valid). A value that is an **array or object** — typically a prior step's structured-output field merged into args — renders as **compact JSON**, so a placeholder like `${args.cited_figures}` hands the model machine-readable data rather than Go's `[map[k:v] ...]` notation. Numbers render in plain decimal (`1000000`, never `1e+06`) whether they sit at the top level or nested inside a composite, and neither form HTML-escapes `<`, `>` or `&`, so URLs with query strings survive intact. Rendering applies to prompts only — `if:` predicates compare against the resolved value's `fmt` form, so predicates on whole arrays/objects are not meaningful (use `sizeof ${args.items}` or a dot-path to a scalar leaf instead). + Step-scoped variables are substituted first so they cannot be shadowed by args of the same name. Arguments accumulate as the flow progresses. When a step produces structured output, its fields are merged into the args map for subsequent steps. `${step.*}` values are **not** merged into args — they exist only for rendering/predicates and do not leak into downstream steps. diff --git a/internal/flow/service.go b/internal/flow/service.go index f484fad0ce..d4cf91ee17 100644 --- a/internal/flow/service.go +++ b/internal/flow/service.go @@ -10,6 +10,7 @@ import ( "maps" "os" "regexp" + "strconv" "strings" "sync" "time" @@ -1930,11 +1931,11 @@ func substituteScoped(template string, args map[string]any, stepVars map[string] } if strings.Contains(template, "${args}") { - argsJSON, err := json.MarshalIndent(args, "", " ") + argsJSON, err := marshalPromptJSON(args, " ") if err != nil { - argsJSON = []byte("{}") + argsJSON = "{}" } - template = strings.ReplaceAll(template, "${args}", string(argsJSON)) + template = strings.ReplaceAll(template, "${args}", argsJSON) } return argsPlaceholderRegex.ReplaceAllStringFunc(template, func(match string) string { @@ -1947,10 +1948,58 @@ func substituteScoped(template string, args map[string]any, stepVars map[string] // behaviour and lets resolveSessionPrefix detect misses. return match } - return fmt.Sprintf("%v", value) + return renderTemplateValue(value) }) } +// renderTemplateValue converts a resolved placeholder value to its prompt +// representation. Strings substitute verbatim, so `"${args.aid}"` in a +// prompt stays valid. +// +// Composite values — a struct-output array/object merged into args, e.g. +// cited_figures — render as compact JSON: fmt's `[map[k:v] ...]` notation +// is lossy (unquoted, ambiguous around spaces) and models mis-read it +// (CD-4975). Only the JSON-shaped types are matched because every args +// value passes through json.Unmarshal (flow-state rows, struct output, +// the /flow/run body) and copyArgs' marshal/unmarshal round-trip, so +// composites are always map[string]any / []any. +// +// That same round-trip makes EVERY args number a float64, and fmt renders +// those with %g — a round million reaches the prompt as `1e+06` and a +// millisecond epoch as `1.7e+12`, while the identical number nested inside +// a composite renders as plain digits through encoding/json. Formatting +// floats with 'f' keeps the scalar and composite paths agreeing. +func renderTemplateValue(value any) string { + switch v := value.(type) { + case map[string]any, []any: + if s, err := marshalPromptJSON(v, ""); err == nil { + return s + } + case float64: + return strconv.FormatFloat(v, 'f', -1, 64) + } + return fmt.Sprintf("%v", value) +} + +// marshalPromptJSON encodes v as JSON destined for a prompt. HTML escaping +// is OFF: json.Marshal rewrites `<`, `>` and `&` as \u003c / \u003e / +// \u0026, which is noise for a model and shows up in ordinary args — URLs +// with query strings (`?a=1&b=2`), claims like "save rate > 25%". An empty +// indent yields the compact single-line form. json.Encoder appends a +// trailing newline that has no place mid-prompt, so strip it. +func marshalPromptJSON(v any, indent string) (string, error) { + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + if indent != "" { + enc.SetIndent("", indent) + } + if err := enc.Encode(v); err != nil { + return "", err + } + return strings.TrimRight(buf.String(), "\n"), nil +} + // resolveArgsPath resolves a dot-path against args. Top-level exact-key // match wins first (preserves backward compatibility with any flat key // that literally contains a dot); otherwise the path is split on `.` diff --git a/internal/flow/service_test.go b/internal/flow/service_test.go index 289d1558d4..56114b5384 100644 --- a/internal/flow/service_test.go +++ b/internal/flow/service_test.go @@ -2,6 +2,7 @@ package flow import ( "encoding/json" + "strings" "testing" ) @@ -179,6 +180,99 @@ func TestSubstituteScoped_BareArgsStillJSON(t *testing.T) { } } +func TestSubstituteScoped_ArrayOfObjectsRendersAsJSON(t *testing.T) { + // A composite value merged into args from a step's struct output + // (e.g. cancel-survey-audit's cited_figures) must render as JSON, + // not fmt's `[map[k:v] ...]` Go notation (CD-4975). + args := map[string]any{ + "cited_figures": []any{ + map[string]any{"claim": "Total cancel events", "value": float64(1882), "unit": "count"}, + map[string]any{"claim": "Client overall save rate", "value": float64(29), "unit": "percent"}, + }, + } + got := substituteScoped("figures: ${args.cited_figures}", args, nil) + want := `figures: [{"claim":"Total cancel events","unit":"count","value":1882},` + + `{"claim":"Client overall save rate","unit":"percent","value":29}]` + if got != want { + t.Errorf("substituteScoped() = %q, want %q", got, want) + } + if containsSubstring(got, "map[") { + t.Errorf("substituteScoped() leaked Go map notation: %q", got) + } +} + +func TestSubstituteScoped_ObjectValueRendersAsJSON(t *testing.T) { + // Same for a nested object resolved as a whole (not via a dot-path + // into a scalar leaf). + args := map[string]any{ + "reviewer": map[string]any{"email": "u@x.com", "name": "U"}, + } + got := substituteScoped("${args.reviewer}", args, nil) + want := `{"email":"u@x.com","name":"U"}` + if got != want { + t.Errorf("substituteScoped() = %q, want %q", got, want) + } +} + +func TestSubstituteScoped_CompositeJSONDoesNotHTMLEscape(t *testing.T) { + // json.Marshal would rewrite `>` and `&` as \u003e / \u0026. Those + // show up in ordinary struct output (URLs with query strings, prose + // comparisons) and are noise in a prompt. + args := map[string]any{ + "cited_figures": []any{ + map[string]any{"claim": "save rate > 25%", "url": "https://x.io/a?b=1&c=2"}, + }, + } + got := substituteScoped("${args.cited_figures}", args, nil) + want := `[{"claim":"save rate > 25%","url":"https://x.io/a?b=1&c=2"}]` + if got != want { + t.Errorf("substituteScoped() = %q, want %q", got, want) + } +} + +func TestSubstituteScoped_BareArgsDoesNotHTMLEscape(t *testing.T) { + args := map[string]any{"url": "https://x.io/a?b=1&c=2"} + got := substituteScoped("${args}", args, nil) + if containsSubstring(got, `\u0026`) { + t.Errorf("bare ${args} HTML-escaped the payload: %q", got) + } + if !containsSubstring(got, `"url": "https://x.io/a?b=1&c=2"`) { + t.Errorf("bare ${args} = %q, want the unescaped URL", got) + } + if strings.HasSuffix(got, "\n") { + t.Errorf("bare ${args} kept the encoder's trailing newline: %q", got) + } +} + +func TestSubstituteScoped_FloatScalarsRenderAsPlainDecimal(t *testing.T) { + // copyArgs' JSON round-trip makes every args number a float64, and + // fmt's %g would emit `1e+06` / `1.7e+12` — while the same number + // nested in a composite renders as plain digits. Both paths must agree. + args := map[string]any{ + "count": float64(1000000), + "timestamp": float64(1699999999999), + "ratio": float64(0.25), + "whole": float64(29), + "nested": map[string]any{"count": float64(1000000)}, + } + tests := []struct { + template string + want string + }{ + {"${args.count}", "1000000"}, + {"${args.timestamp}", "1699999999999"}, + {"${args.ratio}", "0.25"}, + {"${args.whole}", "29"}, + {"${args.nested.count}", "1000000"}, + {"${args.nested}", `{"count":1000000}`}, + } + for _, tt := range tests { + if got := substituteScoped(tt.template, args, nil); got != tt.want { + t.Errorf("substituteScoped(%q) = %q, want %q", tt.template, got, tt.want) + } + } +} + func TestSubstituteArgs(t *testing.T) { tests := []struct { name string