fix(xerrors): keep outer fields when rendering collected errors

internal/x/xerrors/collector.go

@@ -15,17 +15,17 @@ import (
 // Collectors form a hierarchy via [Collector.NewChild]; [Collector.Join]
 // recursively merges own errors with children's.
 //
-// When a child's [collected] error is wrapped (e.g. via [WrapKV]), the
-// outer [withMessage] is remembered and re-applied in [Collector.Join],
-// producing: collected → withMessage{fields} → joinError{…}.
+// When a child's [collected] error is wrapped (e.g. via [WrapKV]), the scope
+// fields of the wrapper layers are remembered and re-applied in
+// [Collector.Join], producing: collected → withMessage{fields} → joinError{…}.
 type Collector struct {
-	mu       sync.Mutex
-	errs     []error
-	children []*Collector
-	counter  atomic.Int32
-	maxErrs  int32
-	parent   *Collector
-	outerWM  *withMessage // outer WrapKV layer, re-applied in Join()
+	mu          sync.Mutex
+	errs        []error
+	children    []*Collector
+	counter     atomic.Int32
+	maxErrs     int32
+	parent      *Collector
+	outerFields map[string]any // scope fields of the outer wrappers, re-applied in Join()
 }
 
 // NewCollector creates a root Collector.
@@ -59,8 +59,8 @@ func normalizeMax(maxErrs int) int32 {
 //
 //   - nil err: no-op unless this collector (or an ancestor) is already full,
 //     in which case the joined error tree is returned immediately.
-//   - already-collected err (*collected): records the outer WrapKV layer for
-//     re-wrapping in Join; does not increment any counter.
+//   - already-collected err (*collected): records the outer wrapper layers'
+//     scope fields for re-wrapping in Join; does not increment any counter.
 //   - ordinary err: increments every ancestor's counter, stores the error if
 //     it is within every ancestor's budget, and returns the joined error tree
 //     if any ancestor has now reached its limit (nil otherwise).
@@ -75,20 +75,22 @@ func (c *Collector) Collect(err error) error {
 	// Already-collected error (from a Join() somewhere): if the collected
 	// error originates from the same collector tree as c (shares the same
 	// root), it is or will be reachable from the root via tree auto-join, so
-	// we must not double-count it here. We only record the outer WrapKV
-	// layer on the originating collector for re-wrapping in its Join().
+	// we must not double-count it here. We only record the outer wrapper
+	// layers' scope fields on the originating collector for re-wrapping in
+	// its Join(). The assignment is unconditional: a wrapper carrying no
+	// scope fields must clear the previously recorded ones instead of
+	// leaving them to be re-applied to this unrelated join.
 	//
 	// Otherwise (foreign collected, e.g. from an unrelated parser-local
 	// fail-fast collector that is not part of c's tree), fall through and
 	// treat it as an ordinary error so it is still stored and counted on
 	// this collector; without this fallback the error would silently vanish.
 	var ce *collected
 	if errors.As(err, &ce) && ce.origin != nil && ce.origin.sameTreeAs(c) {
-		if wm, ok := err.(*withMessage); ok {
-			ce.origin.mu.Lock()
-			ce.origin.outerWM = wm
-			ce.origin.mu.Unlock()
-		}
+		fields := outerScopeFields(err, ce)
+		ce.origin.mu.Lock()
+		ce.origin.outerFields = fields
+		ce.origin.mu.Unlock()
 		if c.IsFull() {
 			return c.Join()
 		}
@@ -162,7 +164,7 @@ func (c *Collector) Join() error {
 	copy(ownErrs, c.errs)
 	kids := make([]*Collector, len(c.children))
 	copy(kids, c.children)
-	outerWM := c.outerWM
+	outerFields := c.outerFields
 	c.mu.Unlock()
 
 	var nonNil []error
@@ -180,16 +182,49 @@ func (c *Collector) Join() error {
 		return nil
 	}
 	var inner error = &joinError{errs: nonNil, stack: callers(1)}
-	// Re-wrap with outer WrapKV fields if present.
-	if outerWM != nil {
-		inner = &withMessage{cause: inner, fields: outerWM.fields}
+	// Re-wrap with the recorded outer scope fields if present.
+	if len(outerFields) > 0 {
+		inner = &withMessage{cause: inner, fields: outerFields}
 	}
 	return &collected{
 		error:  inner,
 		origin: c,
 	}
 }
 
+// outerScopeFields merges the [scopeKeys] fields of every wrapper layer between
+// err and the collected marker ce, with inner layers winning on key conflicts.
+// Returns nil if no layer carries one.
+//
+// All intermediate layers must be walked, not just the outermost one: a wrapper
+// chain often spreads its fields across several layers (e.g. confgen adds
+// Module in one layer and BookName/SheetName in another), so keeping only the
+// outermost layer would silently drop the rest.
+//
+// Non-scope fields are skipped because Join() re-applies these to the whole
+// subtree: a cell position or field name belongs to a single error, and
+// broadcasting it would point sibling errors at a cell they never touched.
+func outerScopeFields(err error, ce *collected) map[string]any {
+	var fields map[string]any
+	for cur := err; cur != nil && cur != error(ce); cur = errors.Unwrap(cur) {
+		fc, ok := cur.(fieldsCarrier)
+		if !ok {
+			continue
+		}
+		// Walking outer -> inner, so inner layers win.
+		for k, v := range fc.Fields() {
+			if !scopeKeys[k] {
+				continue
+			}
+			if fields == nil {
+				fields = make(map[string]any)
+			}
+			fields[k] = v
+		}
+	}
+	return fields
+}
+
 // collected marks an error as already joined. Delegates to the inner error.
 type collected struct {
 	error
@@ -198,6 +233,16 @@ type collected struct {
 
 func (c *collected) Error() string { return c.error.Error() }
 func (c *collected) Unwrap() error { return c.error }
+
+// renderWithFields implements [fieldsRenderer], so that outer fields (e.g.
+// Module, BookName, SheetName added by an enclosing [WrapKV]) are propagated
+// into the joined children instead of being dropped. Without this, rendering
+// falls back to the inner error's Error(), which loses the outer fields and
+// thus renders the default message template rather than the module-specific
+// one (e.g. confgen).
+func (c *collected) renderWithFields(outerFields map[string]any) string {
+	return renderCause(c.error, outerFields)
+}
 func (c *collected) Format(s fmt.State, verb rune) {
 	if f, ok := c.error.(fmt.Formatter); ok {
 		f.Format(s, verb)

internal/x/xerrors/collector_test.go

@@ -726,9 +726,9 @@ func TestCollected_CollectForeignWrappedCollectedIsNotDropped(t *testing.T) {
 	assert.Contains(t, got.Error(), "err 1")
 }
 
-// Collecting a WrapKV'd same-tree collected error records the outer WrapKV
-// layer on the originating collector so Join() can re-apply the fields.
-func TestCollected_CollectSameTreeWrappedCollectedRecordsOuterWM(t *testing.T) {
+// Collecting a WrapKV'd same-tree collected error records the outer wrapper's
+// scope fields on the originating collector so Join() can re-apply them.
+func TestCollected_CollectSameTreeWrappedCollectedRecordsOuterFields(t *testing.T) {
 	root := NewCollector(10)
 	child := root.NewChild(0)
 	_ = child.Collect(fmt.Errorf("err 1"))
@@ -741,8 +741,8 @@ func TestCollected_CollectSameTreeWrappedCollectedRecordsOuterWM(t *testing.T) {
 	// and the outer WrapKV layer is recorded on child (origin) for Join().
 	_ = root.Collect(wrapped)
 
-	if assert.NotNil(t, child.outerWM, "outer WrapKV layer should be recorded on the originating collector") {
-		assert.Equal(t, "test.xlsx", child.outerWM.fields[KeyBookName])
+	if assert.NotNil(t, child.outerFields, "outer scope fields should be recorded on the originating collector") {
+		assert.Equal(t, "test.xlsx", child.outerFields[KeyBookName])
 	}
 
 	// child.Join() re-applies the recorded fields, surfacing them in its Desc.
@@ -787,6 +787,148 @@ func TestCollected_ErrorDelegates(t *testing.T) {
 	assert.Contains(t, joined.Error(), "hello")
 }
 
+// collected marker is transparent to field propagation: outer WrapKV fields
+// must reach the joined children when rendering via Error(), so the
+// module-specific template (confgen) is used instead of the default one.
+//
+// This mirrors the real confgen chain:
+//
+//	parseFieldValue             -> E2002
+//	tableParser.Parse           -> WrapKV(DataCellPos/DataCell/ColumnName)
+//	sheetCollector.Join         -> collected{withMessage{joinError}}
+//	sheetParser.Parse           -> WrapKV(Module)
+//	parseMessageFromOneImporter -> WrapKV(Module/BookName/SheetName)
+func TestCollected_ErrorPropagatesOuterFields(t *testing.T) {
+	cellErr := WrapKV(E2002("100033333", "ItemConf.ID"),
+		KeyDataCellPos, "F12",
+		KeyDataCell, "100033333",
+		KeyColumnName, "ItemID",
+	)
+
+	child := NewCollector(10).NewChild(5)
+	_ = child.Collect(cellErr)
+
+	err := WrapKV(WrapKV(child.Join(), KeyModule, ModuleConf),
+		KeyModule, ModuleConf,
+		KeyBookName, "Activity.xlsx",
+		KeySheetName, "SectionConf",
+	)
+
+	want := `error[E2002]: field value not in referred space
+Workbook: Activity.xlsx
+Worksheet: SectionConf
+DataCellPos: F12
+DataCell: 100033333
+Reason: value "100033333" not in referred space "ItemConf.ID"
+Help: guarantee value "100033333" was configured in referred space "ItemConf.ID" ahead
+`
+	assert.Equal(t, want, err.Error())
+}
+
+// Re-collecting an already-collected same-tree error must preserve the fields
+// of every wrapper layer, not just the outermost one.
+//
+// This mirrors the real confgen chain of a merger/scatter sheet, where Module
+// and BookName/SheetName are added by different layers:
+//
+//	tableParser.Parse           -> sheetCollector.Join() == collected
+//	sheetParser.Parse           -> WrapKV(Module)
+//	parseMessageFromOneImporter -> WrapKV(Module/BookName/SheetName/PBMessage)
+//	ParseMessage's Group.Go     -> WrapKV(BookName/SheetName/Primary*)  <- outermost, no Module
+//	Group.Wait                  -> bookCollector.Join()
+func TestCollected_ReCollectPreservesAllWrapperFields(t *testing.T) {
+	cellErr := WrapKV(E2002("100033333", "ItemConf.ID"),
+		KeyDataCellPos, "F12",
+		KeyDataCell, "100033333",
+	)
+
+	book := NewCollector(10)
+	sheet := book.NewChild(5)
+	_ = sheet.Collect(cellErr)
+
+	// Module is added by an intermediate layer, while the outermost layer only
+	// carries book/sheet names.
+	err := WrapKV(WrapKV(sheet.Join(),
+		KeyModule, ModuleConf,
+		KeyBookName, "Activity.xlsx",
+		KeySheetName, "SectionConf",
+	), KeyPrimaryBookName, "Activity.xlsx", KeyPrimarySheetName, "SectionConf")
+	_ = book.Collect(err)
+
+	want := `error[E2002]: field value not in referred space
+Workbook: Activity.xlsx
+Worksheet: SectionConf
+DataCellPos: F12
+DataCell: 100033333
+Reason: value "100033333" not in referred space "ItemConf.ID"
+Help: guarantee value "100033333" was configured in referred space "ItemConf.ID" ahead
+`
+	assert.Equal(t, want, book.Join().Error())
+}
+
+// Only scope fields (which book/sheet/message) may be broadcast to the joined
+// children; per-cell fields belong to a single error. Recording a wrapper's
+// DataCellPos/DataCell would point every sibling at a cell it never touched.
+//
+// This mirrors the real confgen chain of a horizontal map, where the wrapper
+// around the nested join carries the *key* column's cell:
+//
+//	parseMessage               -> messageCollector.Join() == collected
+//	parseHorizontalMapField    -> WrapKV(CellDebugKV of the key column)
+//	parseMessage (parent)      -> messageCollector.Collect(...)  <- same tree
+func TestCollected_ReCollectDropsNonScopeFields(t *testing.T) {
+	sheet := NewCollector(10)
+	nested := sheet.NewChild(5)
+	// Sibling 1 owns its cell; sibling 2 has no cell of its own.
+	_ = nested.Collect(WrapKV(E2002("100033333", "ItemConf.ID"),
+		KeyDataCellPos, "B4",
+		KeyDataCell, "100033333",
+	))
+	_ = nested.Collect(E2014("Item1Miss"))
+
+	// The wrapper carries the key column's cell alongside the scope fields.
+	_ = sheet.Collect(WrapKV(nested.Join(),
+		KeyModule, ModuleConf,
+		KeyBookName, "Activity.xlsx",
+		KeySheetName, "SectionConf",
+		KeyDataCellPos, "A4",
+		KeyDataCell, "7",
+		KeyColumnName, "Item1ID",
+	))
+
+	got := sheet.Join().Error()
+	// Scope fields are broadcast to both children.
+	assert.Contains(t, got, "Workbook: Activity.xlsx")
+	assert.Contains(t, got, "Worksheet: SectionConf")
+	// Sibling 1 keeps its own cell; the wrapper's cell reaches neither.
+	assert.Contains(t, got, "DataCellPos: B4")
+	assert.NotContains(t, got, "DataCellPos: A4",
+		"wrapper cell position must not be broadcast to the joined children")
+	assert.NotContains(t, got, "DataCell: 7",
+		"wrapper cell data must not be broadcast to the joined children")
+}
+
+// A wrapper carrying no scope fields must clear the previously recorded ones,
+// so a stale book/sheet name is never re-applied to an unrelated join.
+func TestCollected_ReCollectFieldlessWrapperClearsStaleFields(t *testing.T) {
+	book := NewCollector(10)
+	first := book.NewChild(5)
+	_ = first.Collect(E2002("v1", "ItemConf.ID"))
+	_ = book.Collect(WrapKV(book.Join(),
+		KeyModule, ModuleConf,
+		KeyBookName, "First.xlsx",
+		KeySheetName, "S1",
+	))
+
+	second := book.NewChild(5)
+	_ = second.Collect(E2002("v2", "ShopConf.ID"))
+	_ = book.Collect(Wrap(book.Join()))
+
+	got := book.Join().Error()
+	assert.NotContains(t, got, "First.xlsx", "stale book name must not survive")
+	assert.NotContains(t, got, "Worksheet: S1", "stale sheet name must not survive")
+}
+
 // collected marker is transparent: errors.Is works through it.
 func TestCollected_ErrorsIsWorksThrough(t *testing.T) {
 	target := fmt.Errorf("target")

internal/x/xerrors/desc.go

@@ -80,6 +80,22 @@ var keys = []string{
 	keyHelp,
 }
 
+// scopeKeys are the keys identifying the enclosing scope (which dir, book,
+// sheet, message) rather than a location inside it. Only these may be shared
+// across the errors of a joined tree: a cell position or field name describes
+// one error alone, so broadcasting it would point the others at a wrong cell.
+var scopeKeys = map[string]bool{
+	KeyModule:           true,
+	KeyIndir:            true,
+	KeySubdir:           true,
+	KeyOutdir:           true,
+	KeyBookName:         true,
+	KeyPrimaryBookName:  true,
+	KeySheetName:        true,
+	KeyPrimarySheetName: true,
+	KeyPBMessage:        true,
+}
+
 // multiUnwrapper is implemented by joined errors (e.g. errors.Join).
 type multiUnwrapper interface {
 	Unwrap() []error

internal/x/xerrors/errors.go

@@ -135,13 +135,7 @@ func (b *base) Format(s fmt.State, verb rune) {
 
 // renderWithFields delegates to the cause, passing outerFields through the stack wrapper.
 func (b *base) renderWithFields(outerFields map[string]any) string {
-	if b.cause != nil {
-		if r, ok := b.cause.(fieldsRenderer); ok {
-			return r.renderWithFields(outerFields)
-		}
-		return b.cause.Error()
-	}
-	return ""
+	return renderCause(b.cause, outerFields)
 }
 
 // withMessage wraps a cause with an optional message and structured fields.
@@ -165,6 +159,18 @@ type fieldsRenderer interface {
 	renderWithFields(outerFields map[string]any) string
 }
 
+// renderCause renders cause with outerFields propagated into it, falling back
+// to plain Error() for causes that do not carry fields.
+func renderCause(cause error, outerFields map[string]any) string {
+	if cause == nil {
+		return ""
+	}
+	if r, ok := cause.(fieldsRenderer); ok {
+		return r.renderWithFields(outerFields)
+	}
+	return cause.Error()
+}
+
 func (w *withMessage) Error() string {
 	if w.message != "" {
 		// replacesCause: message is the complete text.
@@ -205,13 +211,7 @@ func (w *withMessage) renderWithFields(outerFields map[string]any) string {
 	merged := make(map[string]any, len(outerFields)+len(w.fields))
 	maps.Copy(merged, outerFields)
 	maps.Copy(merged, w.fields)
-	if w.cause != nil {
-		if r, ok := w.cause.(fieldsRenderer); ok {
-			return r.renderWithFields(merged)
-		}
-		return w.cause.Error()
-	}
-	return ""
+	return renderCause(w.cause, merged)
 }
 
 func (w *withMessage) Format(s fmt.State, verb rune) {