fix: preserve key order in YAML output (#2034)

* fix: preserve key order in YAML output

MlrmapToYAMLNative converted the ordered Mlrmap to map[string]interface{},
which loses insertion order. yaml.v3.Marshal then sorts Go map keys
alphabetically, so YAML output always had sorted keys despite other formats
(JSON, pretty-print) correctly preserving order.

Switch to building yaml.Node trees that preserve the Mlrmap's linked-list
iteration order. Use yaml.Node.Encode for scalar values to correctly handle
edge cases (NaN/Inf, non-UTF-8 strings).

Fixes #2028.

* Update pkg/mlrval/mlrval_yaml_test.go with copilot code-review feedback

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: lawrence3699 <lawrence3699@users.noreply.github.com>
Co-authored-by: John Kerl <kerl.john.r@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

pkg/mlrval/mlrval_yaml.go

@@ -138,63 +138,79 @@ func mlrvalFromYAMLArray(a []interface{}) (*Mlrval, error) {
 	return out, nil
 }
 
-// MlrmapToYAMLNative converts an Mlrmap to a Go value suitable for
-// yaml.Marshal: map[string]interface{} with nested maps/arrays.
-func MlrmapToYAMLNative(mlrmap *Mlrmap) (interface{}, error) {
+// MlrmapToYAMLNative converts an Mlrmap to a *yaml.Node that preserves
+// key insertion order. The returned node is suitable for yaml.Marshal.
+func MlrmapToYAMLNative(mlrmap *Mlrmap) (*yaml.Node, error) {
 	if mlrmap == nil {
-		return nil, nil
+		return &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"}, nil
+	}
+	node := &yaml.Node{
+		Kind: yaml.MappingNode,
+		Tag:  "!!map",
 	}
-	out := make(map[string]interface{})
 	for pe := mlrmap.Head; pe != nil; pe = pe.Next {
-		v, err := mlrvalToYAMLNative(pe.Value)
+		keyNode, err := encodeScalarNode(pe.Key)
 		if err != nil {
 			return nil, err
 		}
-		out[pe.Key] = v
+		valNode, err := mlrvalToYAMLNode(pe.Value)
+		if err != nil {
+			return nil, err
+		}
+		node.Content = append(node.Content, keyNode, valNode)
 	}
-	return out, nil
+	return node, nil
 }
 
-// mlrvalToYAMLNative converts *Mlrval to a Go value for yaml.Marshal.
-func mlrvalToYAMLNative(mv *Mlrval) (interface{}, error) {
+// encodeScalarNode creates a yaml.Node by delegating to yaml.v3's Encode,
+// which handles edge cases like NaN/Inf floats and non-UTF-8 strings.
+func encodeScalarNode(v interface{}) (*yaml.Node, error) {
+	node := &yaml.Node{}
+	if err := node.Encode(v); err != nil {
+		return nil, err
+	}
+	return node, nil
+}
+
+// mlrvalToYAMLNode converts *Mlrval to a *yaml.Node for yaml.Marshal.
+func mlrvalToYAMLNode(mv *Mlrval) (*yaml.Node, error) {
 	if mv == nil {
-		return nil, nil
+		return encodeScalarNode(nil)
 	}
 	switch mv.Type() {
 	case MT_ABSENT, MT_VOID, MT_NULL:
-		return nil, nil
+		return encodeScalarNode(nil)
 	case MT_STRING:
 		s, _ := mv.GetStringValue()
-		return s, nil
+		return encodeScalarNode(s)
 	case MT_INT:
 		i, _ := mv.GetIntValue()
-		return i, nil
+		return encodeScalarNode(i)
 	case MT_FLOAT:
 		f, _ := mv.GetFloatValue()
-		return f, nil
+		return encodeScalarNode(f)
 	case MT_BOOL:
 		b, _ := mv.GetBoolValue()
-		return b, nil
+		return encodeScalarNode(b)
 	case MT_ARRAY:
 		arr := mv.GetArray()
-		if arr == nil {
-			return []interface{}{}, nil
-		}
-		out := make([]interface{}, 0, len(arr))
-		for _, elem := range arr {
-			v, err := mlrvalToYAMLNative(elem)
-			if err != nil {
-				return nil, err
+		seqNode := &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq"}
+		if arr != nil {
+			for _, elem := range arr {
+				v, err := mlrvalToYAMLNode(elem)
+				if err != nil {
+					return nil, err
+				}
+				seqNode.Content = append(seqNode.Content, v)
 			}
-			out = append(out, v)
 		}
-		return out, nil
+		return seqNode, nil
 	case MT_MAP:
 		m := mv.GetMap()
 		return MlrmapToYAMLNative(m)
 	case MT_ERROR, MT_PENDING:
-		return mv.String(), nil
+		return encodeScalarNode(mv.String())
 	default:
-		return mv.String(), nil
+		return encodeScalarNode(mv.String())
 	}
 }

pkg/mlrval/mlrval_yaml_test.go

@@ -177,6 +177,38 @@ func TestYAMLKeyStringNonStringKeys(t *testing.T) {
 	assert.True(t, v.IsInt())
 }
 
+func TestMlrmapToYAMLNativeKeyOrder(t *testing.T) {
+	rec := NewMlrmapAsRecord()
+	rec.PutReference("b", FromInt(22))
+	rec.PutReference("c", FromInt(33))
+	rec.PutReference("a", FromInt(11))
+
+	native, err := MlrmapToYAMLNative(rec)
+	assert.NoError(t, err)
+	out, err := yaml.Marshal(native)
+	assert.NoError(t, err)
+
+	var node yaml.Node
+	err = yaml.Unmarshal(out, &node)
+	assert.NoError(t, err)
+	if !assert.Len(t, node.Content, 1, "expected a single YAML document node") {
+		return
+	}
+
+	mapping := node.Content[0]
+	if !assert.Equal(t, yaml.MappingNode, mapping.Kind, "expected top-level YAML mapping") {
+		return
+	}
+
+	// Keys must appear in insertion order (b, c, a), not sorted (a, b, c).
+	if !assert.GreaterOrEqual(t, len(mapping.Content), 6, "expected at least three key/value pairs") {
+		return
+	}
+	assert.Equal(t, "b", mapping.Content[0].Value)
+	assert.Equal(t, "c", mapping.Content[2].Value)
+	assert.Equal(t, "a", mapping.Content[4].Value)
+}
+
 func TestMlrmapToYAMLNativeNested(t *testing.T) {
 	inner := NewMlrmapAsRecord()
 	inner.PutReference("p", FromInt(1))

pkg/output/record_writer_yaml.go

@@ -14,8 +14,8 @@ import (
 
 type RecordWriterYAML struct {
 	writerOptions   *cli.TWriterOptions
-	bufferedRecords []interface{} // used when WrapYAMLOutputInOuterList is true
-	wroteAnyRecords bool          // for multi-doc: emit "---\n" before 2nd and later docs
+	bufferedRecords []*yaml.Node // used when WrapYAMLOutputInOuterList is true
+	wroteAnyRecords bool         // for multi-doc: emit "---\n" before 2nd and later docs
 }
 
 func NewRecordWriterYAML(writerOptions *cli.TWriterOptions) (*RecordWriterYAML, error) {
@@ -46,7 +46,7 @@ func (writer *RecordWriterYAML) writeWithListWrap(
 ) {
 	if outrec != nil {
 		if writer.bufferedRecords == nil {
-			writer.bufferedRecords = []interface{}{}
+			writer.bufferedRecords = []*yaml.Node{}
 		}
 		native, err := mlrval.MlrmapToYAMLNative(outrec)
 		if err != nil {
@@ -56,7 +56,9 @@ func (writer *RecordWriterYAML) writeWithListWrap(
 		writer.bufferedRecords = append(writer.bufferedRecords, native)
 	} else {
 		// End of stream: emit single YAML document as array
-		out, err := yaml.Marshal(writer.bufferedRecords)
+		seqNode := &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq"}
+		seqNode.Content = writer.bufferedRecords
+		out, err := yaml.Marshal(seqNode)
 		if err != nil {
 			fmt.Fprintf(os.Stderr, "mlr: %v\n", err)
 			os.Exit(1)