fix(confgen): preserve sub-table book/sheet name in merger errors

.github/workflows/buf-ci.yml

@@ -19,5 +19,6 @@ jobs:
       - uses: actions/checkout@v6
       - uses: bufbuild/buf-action@v1
         with:
+          version: "1.69.0"
           token: ${{ secrets.BUF_TOKEN }}
           github_token: ${{ secrets.GITHUB_TOKEN }}

internal/confgen/collector_integration_test.go

@@ -181,3 +181,62 @@ func TestCollectorIntegration_MultiBookCapped(t *testing.T) {
 		assert.Contains(t, got, e2012("Collector2#*.csv", "HeroConf", cells[i], v, "uint32"))
 	}
 }
+
+// TestCollectorIntegration_MergerSubtableBookName verifies that when a
+// SHARD sub-table fails to parse inside the ParseMessage merger goroutine,
+// the rendered error reports the offending shard's own BookName/SheetName,
+// and additionally annotates the primary main workbook as "(Primary: ...)".
+//
+// Layout:
+//   - main workbook "MergerCollector#*.csv" / sheet MergerCollectorItemConf
+//     contains only valid rows.
+//   - shard workbook  "MergerShard1#*.csv"   / sheet MergerCollectorItemConf
+//     contains one invalid Num cell ("bad_num") that triggers E2012.
+//
+// The shard is merged via `merger: "MergerShard*.csv#MergerCollectorItemConf"`
+// in merger.proto. parseMessageFromOneImporter fails on the shard, the
+// merger goroutine WrapKV's the error with both the shard names AND the
+// primary names; the fix ensures the shard names survive into the final
+// error description (regression: previously only the primary names were
+// kept and the user could not tell which shard actually broke).
+//
+// Note: uses a separate proto package + testdata tree so its workbook
+// options don't perturb the other TestCollectorIntegration_* tests above.
+func TestCollectorIntegration_MergerSubtableBookName(t *testing.T) {
+	const mergerProtoDir = "./testdata/collector_merger/proto"
+	gen := NewGenerator("collectormergertest",
+		"./testdata/collector_merger/csv/merger/",
+		"./testdata/_collector_merger_out/",
+		options.Conf(
+			&options.ConfOption{
+				Input: &options.ConfInputOption{
+					ProtoPaths: []string{mergerProtoDir},
+					ProtoFiles: []string{mergerProtoDir + "/*.proto"},
+					Formats:    []format.Format{format.CSV},
+				},
+				Output: &options.ConfOutputOption{
+					Formats: []format.Format{format.JSON},
+				},
+			},
+		),
+	)
+	err := gen.Generate("MergerCollector#MergerCollectorItemConf.csv")
+	require.Error(t, err)
+
+	got := err.Error()
+	t.Logf("got error string:\n%s", got)
+
+	// The offending cell ("bad_num") must be reported against the SHARD
+	// workbook, not the main workbook.
+	assert.Contains(t, got, "Workbook: MergerShard1#*.csv",
+		"shard BookName must appear in the rendered error")
+	assert.Contains(t, got, "Worksheet: MergerCollectorItemConf",
+		"shard SheetName must appear in the rendered error")
+	// And the main workbook must be annotated as Primary, proving both
+	// (BookName, PrimaryBookName) survived WrapKV layering.
+	assert.Contains(t, got, "(Primary: MergerCollector#*.csv)",
+		"primary BookName must be annotated alongside the shard BookName")
+	// The error must NOT point at the main workbook as the offending one.
+	assert.NotContains(t, got, "Workbook: MergerCollector#*.csv ",
+		"main workbook must not be reported as the offending file")
+}

internal/confgen/parser.go

@@ -158,17 +158,21 @@ func ParseMessage(info *SheetInfo, collector *xerrors.Collector, impInfos ...imp
 	for _, impInfo := range impInfos {
 		// map-reduce: map jobs for concurrent processing
 		g.Go(func(ctx context.Context) error {
+			bookName := getRelBookName(info.ExtInfo.InputDir, impInfo.Filename())
+			sheetName := getRealSheetName(info, impInfo)
 			protomsg, err := parseMessageFromOneImporter(info, collector, impInfo)
 			if err != nil {
 				return xerrors.WrapKV(err,
+					xerrors.KeyBookName, bookName,
+					xerrors.KeySheetName, sheetName,
 					xerrors.KeyPrimaryBookName, info.PrimaryBookName,
 					xerrors.KeyPrimarySheetName, info.SheetOpts.Name)
 			}
 			mu.Lock()
 			msgs = append(msgs, oneMsg{
 				protomsg:  protomsg,
-				bookName:  getRelBookName(info.ExtInfo.InputDir, impInfo.Filename()),
-				sheetName: getRealSheetName(info, impInfo),
+				bookName:  bookName,
+				sheetName: sheetName,
 			})
 			mu.Unlock()
 			return nil

internal/confgen/testdata/collector_merger/csv/merger/MergerCollector#@TABLEAU.csv

@@ -0,0 +1,2 @@
+Sheet,Merger
+MergerCollectorItemConf,"MergerShard*.csv#MergerCollectorItemConf"

internal/confgen/testdata/collector_merger/csv/merger/MergerCollector#MergerCollectorItemConf.csv

@@ -0,0 +1,5 @@
+ID,Num
+uint32,int32
+Item's ID,Item's num
+1,10
+2,20

internal/confgen/testdata/collector_merger/csv/merger/MergerShard1#MergerCollectorItemConf.csv

@@ -0,0 +1,4 @@
+ID,Num
+uint32,int32
+Item's ID,Item's num
+3,bad_num

internal/confgen/testdata/collector_merger/proto/merger.proto

@@ -0,0 +1,31 @@
+// clang-format off
+
+syntax = "proto3";
+
+package collectormergertest;
+
+option (tableau.workbook) = {name: "MergerCollector#*.csv"};
+
+import "tableau/protobuf/tableau.proto";
+
+// MergerCollectorItemConf merges the main workbook with one or more shard
+// workbooks matching "MergerShard*.csv#MergerCollectorItemConf". It is used
+// to verify that, when a shard sub-table fails to parse, the collected
+// error reports the offending shard's BookName/SheetName instead of the
+// primary main workbook's names.
+message MergerCollectorItemConf {
+  option (tableau.worksheet) = {
+    name: "MergerCollectorItemConf"
+    namerow: 1
+    typerow: 2
+    noterow: 3
+    datarow: 4
+    merger: "MergerShard*.csv#MergerCollectorItemConf"
+  };
+
+  map<uint32, Item> item_map = 1 [(tableau.field) = {key:"ID" layout:LAYOUT_VERTICAL}];
+  message Item {
+    uint32 id = 1 [(tableau.field) = {name:"ID"}];
+    int32 num = 2 [(tableau.field) = {name:"Num"}];
+  }
+}