From 9c5e19c225d80c7ca0e06ff844a2ff5320a046ec Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 14 Sep 2026 00:40:40 +0000 Subject: [PATCH] Preserve explicit worker counts when normalizing keys --- docs/roadmap/remediation.md | 3 ++ internal/core/config/operational_document.go | 14 ++++++- .../core/config/operational_document_test.go | 40 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 870371ad..c8b4218d 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -100,6 +100,9 @@ chunk/extract/merge/normalize and valid reference forms remain accepted. ## Finding 3 — Restore Worker-Key Normalization Without Losing Explicitness +Status: Addressed. Worker keys normalize before defaulting; original input +presence preserves explicit counts. Regression tests cover defaults and collisions. + Priority: P2. The migration introduced an unplanned restriction. The former `normalizeStageWorkers` trimmed keys and rejected normalized diff --git a/internal/core/config/operational_document.go b/internal/core/config/operational_document.go index 2d18d0fd..192fe8ad 100644 --- a/internal/core/config/operational_document.go +++ b/internal/core/config/operational_document.go @@ -18,7 +18,12 @@ func resolveDocumentOperations(document *Document, resolution *configkit.Resolut normalizeDocumentState(document, resolution) normalizeDocumentPipelines(document, resolution, state) normalizeDocumentPaths(document, resolution, state) - deriveDocumentExtractWorkers(document, resolution) + if document.Concurrency != nil { + document.Concurrency.StageWorkers = normalizeDocumentMap(document.Concurrency.StageWorkers, + configkit.Keys("concurrency", "stage_workers"), "stage worker identifier", + documentNormalizer{Resolution: resolution}, state) + } + deriveDocumentExtractWorkers(document, resolution, state) return nil } @@ -74,7 +79,7 @@ func normalizeDocumentString(resolution *configkit.Resolution, path configkit.Pa configkit.Normalize(resolution, path, target, documentValue(strings.TrimSpace(**target)), reason) } -func deriveDocumentExtractWorkers(document *Document, resolution *configkit.Resolution) { +func deriveDocumentExtractWorkers(document *Document, resolution *configkit.Resolution, state *documentLoadState) { if document.Concurrency == nil || document.Concurrency.TotalLLM == nil { return } @@ -82,6 +87,11 @@ func deriveDocumentExtractWorkers(document *Document, resolution *configkit.Reso document.Concurrency.StageWorkers = make(map[string]int) } path := configkit.Keys("concurrency", "stage_workers", "extract") + // Presence belongs to the input spelling, not the key produced by trimming. + // A moved explicit count must never be replaced by a derived default. + if resolution.IsExplicit(state.originalPath(path)) { + return + } workers := document.Concurrency.StageWorkers value := workers["extract"] configkit.Default(resolution, path, &value, *document.Concurrency.TotalLLM, diff --git a/internal/core/config/operational_document_test.go b/internal/core/config/operational_document_test.go index d15f5355..d5fe2b2c 100644 --- a/internal/core/config/operational_document_test.go +++ b/internal/core/config/operational_document_test.go @@ -1,14 +1,54 @@ package config import ( + "bytes" "errors" "os" "path/filepath" + "strings" "testing" configkit "gitea.maximumdirect.net/eric/ConfigKit" ) +func TestWorkerKeyNormalizationPreservesExplicitCount(t *testing.T) { + t.Parallel() + for _, test := range []struct { + name, workers string + want int + errorPath string + }{ + {name: "omitted", workers: "{}", want: 4}, + {name: "padded", workers: "{' extract ': 2}", want: 2}, + {name: "explicit invalid", workers: "{' extract ': 0}", errorPath: "/concurrency/stage_workers/extract"}, + {name: "collision", workers: "{' extract ': 2, extract: 3}", errorPath: "/concurrency/stage_workers/extract"}, + } { + t.Run(test.name, func(t *testing.T) { + loaded, err := loadConfigSource(configkit.Bytes("workers.yml", []byte( + "version: 5\nconcurrency:\n total_llm: 4\n stage_workers: "+test.workers+"\n")), LoadOptions{}) + if test.errorPath != "" { + assertDocumentDiagnosticAt(t, err, test.errorPath) + return + } + if err != nil { + t.Fatal(err) + } + if got := loaded.Config.Concurrency.StageWorkers["extract"]; got != test.want { + t.Fatalf("workers = %d, want %d", got, test.want) + } + var explanation bytes.Buffer + if err := loaded.WriteExplanation(&explanation, "/concurrency/stage_workers"); err != nil { + t.Fatal(err) + } + if test.name == "padded" && (!strings.Contains(explanation.String(), "normalization") || + !strings.Contains(explanation.String(), " extract ") || + strings.Contains(explanation.String(), "derived extraction workers")) { + t.Fatalf("explicit worker provenance = %s", explanation.String()) + } + }) + } +} + func TestDocumentLoadAppliesOperationalDefaultsAndNormalization(t *testing.T) { t.Parallel()