From 3223b5cb825e475e7dbd35c46163897d0c3c8a64 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 7 Nov 2026 12:06:50 +0000 Subject: [PATCH] Preserve YAML string types and explicit numeric evidence --- docs/roadmap/remediation.md | 13 +++++++++--- report_value.go | 24 +++++++++------------ report_value_test.go | 42 +++++++++++++++++++++++++++++++++---- 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 9d9ca96..2046ab6 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Shared reporting and PromptKit adoption remediation -Status: four open findings; R1–R3 complete. R1–R3 are from the architectural review of +Status: three open findings; R1–R4 complete. R1–R3 are from the architectural review of `f83866e44a8a9066db52cbd2aa5cccde0d69a839`. R4–R7 are from the algorithmic audit of `f5b2d1f80c4720b762f3301c29d5050710c210d0`; R4 is a correctness issue and R5–R7 are efficiency opportunities for consideration. @@ -165,6 +165,13 @@ selected; their implementations below were not prescribed by the plan. ## R4 — P2: YAML conversion reclassifies native strings as numbers +**Complete.** Decoding now honors resolved string tags; core YAML emission +explicitly retains numeric tags for overflow tokens instead of relying on +heuristic decoding. String values/keys and explicit large numbers round-trip +through YAML and JSON. The independent core suite passes, and temporary native +ConfigKit consumer probes pass for whole/focused display and detailed difference +snapshots with Go 1.27.0. No optional-library dependency was added to core. + **Location:** [report_value.go](../../report_value.go), yamlReportValue lines 106–118. @@ -190,8 +197,8 @@ The shared decoder also serves focused display and difference snapshots. problem is type correctness, not its running time. Preserve native resolved string tags rather than inferring numeric intent from spelling. Continue to preserve numeric precision when numeric evidence exists, such as `!!int` or -`!!float` nodes and the core's JSON number representation. Core-generated YAML -already retains explicit numeric tags when needed. Do not compensate by changing +`!!float` nodes and the core's JSON number representation. Retain explicit numeric tags in core-generated YAML when needed; the emitter +can otherwise omit them for overflow tokens. Do not compensate by changing upstream loading or by reparsing mutable runtime configuration. Review tests that currently require numeric promotion of resolved plain strings; they cannot override the selected string-versus-number preservation contract. diff --git a/report_value.go b/report_value.go index 61fe775..475cb31 100644 --- a/report_value.go +++ b/report_value.go @@ -33,7 +33,8 @@ func normalizeReport(value any) (any, error) { // DecodeYAMLReport decodes one YAML document from safe inspection output into // JSON-compatible values. Unsupported YAML meanings return an error; the result -// is not a lossless representation of the input document. +// is not a lossless representation of the input document. Scalar types follow +// the resolved YAML tags; numeric-looking strings remain strings. func DecodeYAMLReport(reader io.Reader) (any, error) { decoder := yaml.NewDecoder(reader) var doc yaml.Node @@ -104,18 +105,6 @@ func yamlReportValue(node *yaml.Node, visiting map[*yaml.Node]bool) (any, error) case yaml.ScalarNode: switch node.ShortTag() { case "!!str": - // The YAML resolver falls back to a string when a numeric scalar is - // outside its machine-number range. Preserve explicit and quoted - // strings, but do not silently turn a plain numeric scalar into text. - if node.Style == 0 { - plain := strings.ReplaceAll(node.Value, "_", "") - if yamlInteger.MatchString(plain) { - return yamlReportNumber(node.Value, "!!int") - } - if yamlNumericPlain.MatchString(plain) { - return yamlReportNumber(node.Value, "!!float") - } - } return node.Value, nil case "!!null": return nil, nil @@ -188,7 +177,14 @@ func reportYAMLNode(value any) (*yaml.Node, error) { if strings.ContainsAny(string(v), ".eE") { tag = "!!float" } - return &yaml.Node{Kind: yaml.ScalarNode, Tag: tag, Value: string(v)}, nil + node := &yaml.Node{Kind: yaml.ScalarNode, Tag: tag, Value: string(v)} + // The YAML emitter can omit numeric tags even when its resolver would + // classify an overflowing token as a string. Retain the numeric evidence + // explicitly; ParseFloat selects only the style, never the stored value. + if _, err := strconv.ParseFloat(string(v), 64); err != nil { + node.Style = yaml.TaggedStyle + } + return node, nil case []any: node := &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq"} for _, item := range v { diff --git a/report_value_test.go b/report_value_test.go index 1fba8fe..c5bb9b6 100644 --- a/report_value_test.go +++ b/report_value_test.go @@ -39,7 +39,7 @@ func TestNormalizeReport(t *testing.T) { } func TestYAMLReportValues(t *testing.T) { - input := `large: 123456789012345678901234567890 + input := `large: !!int 123456789012345678901234567890 hex: 0xff fraction: .125 text: "false" @@ -94,8 +94,8 @@ func TestDecodeYAMLReportRejectsUnsupportedValues(t *testing.T) { func TestYAMLReportLargeNumericScalars(t *testing.T) { for _, tc := range []struct{ input, want string }{ - {"1e9999", "1e9999"}, {"0x100000000000000000000", "1208925819614629174706176"}, - {"+01.2500e+9999", "1.2500e+9999"}, + {"!!float 1e9999", "1e9999"}, {"!!int 0x100000000000000000000", "1208925819614629174706176"}, + {"!!float +01.2500e+9999", "1.2500e+9999"}, } { t.Run(tc.input, func(t *testing.T) { got, err := DecodeYAMLReport(strings.NewReader(tc.input)) @@ -104,7 +104,7 @@ func TestYAMLReportLargeNumericScalars(t *testing.T) { } }) } - for _, input := range []string{`"1e9999"`, `!!str 1e9999`, `.`, `hello`} { + for _, input := range []string{`1e9999`, `"1e9999"`, `!!str 1e9999`, `.`, `hello`} { got, err := DecodeYAMLReport(strings.NewReader(input)) if err != nil { t.Fatal(err) @@ -133,3 +133,37 @@ func TestReportYAMLNodePreservesNumericLookingStrings(t *testing.T) { t.Fatalf("%s: got %#v, want %#v", data, got, value) } } + +func TestDecodeYAMLReportPreservesPlainNativeStrings(t *testing.T) { + // Native ConfigKit safe output can contain an unquoted numeric-looking string + // when the YAML resolver considers the token outside the numeric range. + value, err := DecodeYAMLReport(strings.NewReader(`1e400: 1e400 +quoted: "1e400" +explicit: !!str 1e400 +number: !!float 1e400 +`)) + if err != nil { + t.Fatal(err) + } + want := map[string]any{"1e400": "1e400", "quoted": "1e400", "explicit": "1e400", "number": json.Number("1e400")} + if !reflect.DeepEqual(value, want) { + t.Fatalf("decoded=%#v want=%#v", value, want) + } + for _, format := range []Format{FormatJSON, FormatYAML} { + var output bytes.Buffer + if err := RenderReport(&output, Report{Value: value}, format); err != nil { + t.Fatal(err) + } + var got any + if format == FormatJSON { + decoder := json.NewDecoder(&output) + decoder.UseNumber() + err = decoder.Decode(&got) + } else { + got, err = DecodeYAMLReport(&output) + } + if err != nil || !reflect.DeepEqual(got, want) { + t.Fatalf("format %v: got=%#v err=%v", format, got, err) + } + } +}