From 4b55628ac8ad3d58aa4defd393bc61d65160f61a Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Wed, 13 May 2026 19:32:41 +0200 Subject: [PATCH] Explicit masterfeats/feats removal --- TOPDATA_HARDENING_CONTRACT.md | 20 ++++--- internal/topdata/MASTERFEATS_CONTRACT.md | 4 +- internal/topdata/native.go | 12 ++++- internal/topdata/topdata.go | 28 +++------- internal/topdata/topdata_test.go | 69 ++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 31 deletions(-) diff --git a/TOPDATA_HARDENING_CONTRACT.md b/TOPDATA_HARDENING_CONTRACT.md index 40db2ab..587f358 100644 --- a/TOPDATA_HARDENING_CONTRACT.md +++ b/TOPDATA_HARDENING_CONTRACT.md @@ -42,12 +42,16 @@ One concrete example already observed: - validation in `internal/topdata/topdata.go` still hard-fails specifically for `topdata/data/masterfeats/base.json` if `output` is missing -That mismatch proves the current methodology is stale: +That mismatch proved the current methodology was stale: - the runtime/build side treats `masterfeats` like a generic dataset - the validator still treats it like a special namespace with explicit required metadata +Current status: the output-name mismatch has been fixed for `masterfeats` and +the equivalent `feat` base dataset rule by sharing the derived output-name rule +between discovery and validation. + This same category of problem appears in several other places. ## Findings From Current Audit @@ -67,17 +71,17 @@ This same category of problem appears in several other places. Some of these are legitimate dataset-shape differences. Others are stale special-cases that should be generalized. -### 2. `masterfeats` still carries outdated explicit validation rules +### 2. `masterfeats` carries dataset-specific invariants -`validateMasterfeatsBaseFile()` currently requires: +`validateMasterfeatsBaseFile()` currently enforces: -- `output == masterfeats.2da` +- explicit `output == masterfeats.2da` when `output` is authored - specific required columns - `masterfeats:` row keys The key-prefix and column requirements may still be valid dataset invariants. -The `output` requirement is not clearly justified anymore when generic dataset -discovery already computes a default output name from location. +The stale requirement that `output` must be authored has been removed; validation +now shares the same derived default output-name rule as native discovery. ### 3. Migration still seeds explicit dataset exceptions @@ -128,7 +132,9 @@ keep inheriting brittle assumptions. ### Work Item 1: Remove outdated `masterfeats.output` hard requirement -Investigate and likely change `validateMasterfeatsBaseFile()` so: +Status: implemented. + +`validateMasterfeatsBaseFile()` now: - missing `output` is accepted when generic dataset discovery can derive the same output name deterministically diff --git a/internal/topdata/MASTERFEATS_CONTRACT.md b/internal/topdata/MASTERFEATS_CONTRACT.md index 35ad700..b7204b1 100644 --- a/internal/topdata/MASTERFEATS_CONTRACT.md +++ b/internal/topdata/MASTERFEATS_CONTRACT.md @@ -44,7 +44,9 @@ Canonical shape: Rules: -- `output` must be exactly `masterfeats.2da` +- `output` may be omitted; native dataset discovery deterministically derives + `masterfeats.2da` from `topdata/data/masterfeats/base.json` +- if `output` is present, it must be exactly `masterfeats.2da` - `columns` must include `LABEL`, `STRREF`, and `DESCRIPTION` - every canonical row must have a non-empty `key` - row keys must start with `masterfeats:` diff --git a/internal/topdata/native.go b/internal/topdata/native.go index d22e3cc..5f76564 100644 --- a/internal/topdata/native.go +++ b/internal/topdata/native.go @@ -527,7 +527,7 @@ func discoverNativeDatasets(dataDir string) ([]nativeDataset, error) { } outputName, _ := baseData["output"].(string) if strings.TrimSpace(outputName) == "" { - outputName = filepath.Base(path) + ".2da" + outputName = nativeDatasetDefaultOutputName(path, nativeDatasetBase) } datasets = append(datasets, nativeDataset{ Kind: nativeDatasetBase, @@ -561,7 +561,7 @@ func discoverNativeDatasets(dataDir string) ([]nativeDataset, error) { } outputName, _ := tableData["output"].(string) if strings.TrimSpace(outputName) == "" { - outputName = strings.TrimSuffix(name, filepath.Ext(name)) + ".2da" + outputName = nativeDatasetDefaultOutputName(filePath, nativeDatasetPlain) } datasets = append(datasets, nativeDataset{ Kind: nativeDatasetPlain, @@ -4208,6 +4208,14 @@ func outputStem(outputName string) string { return strings.TrimSuffix(outputName, filepath.Ext(outputName)) } +func nativeDatasetDefaultOutputName(path string, kind nativeDatasetKind) string { + name := filepath.Base(path) + if kind == nativeDatasetPlain { + name = strings.TrimSuffix(name, filepath.Ext(name)) + } + return name + ".2da" +} + func loadJSONObject(path string) (map[string]any, error) { raw, err := os.ReadFile(path) if err != nil { diff --git a/internal/topdata/topdata.go b/internal/topdata/topdata.go index 21035c8..191db9e 100644 --- a/internal/topdata/topdata.go +++ b/internal/topdata/topdata.go @@ -456,20 +456,7 @@ func validateDataObject(path string, obj map[string]any, report *ValidationRepor func validateMasterfeatsBaseFile(path string, obj map[string]any, report *ValidationReport) { validateRowsFile(path, obj, report, true) - - if output, ok := obj["output"]; !ok { - report.Diagnostics = append(report.Diagnostics, Diagnostic{ - Severity: SeverityError, - Path: path, - Message: "masterfeats base.json must contain output set to masterfeats.2da", - }) - } else if outputText, ok := output.(string); !ok || strings.TrimSpace(outputText) != "masterfeats.2da" { - report.Diagnostics = append(report.Diagnostics, Diagnostic{ - Severity: SeverityError, - Path: path, - Message: "masterfeats output must be masterfeats.2da", - }) - } + validateDerivedBaseOutput(path, obj, "masterfeats", report) columns := extractValidationColumns(obj) for _, required := range []string{"LABEL", "STRREF", "DESCRIPTION"} { @@ -520,18 +507,17 @@ func validateMasterfeatsLockFile(path string, obj map[string]any, report *Valida func validateFeatBaseFile(path string, obj map[string]any, report *ValidationReport) { validateRowsFile(path, obj, report, true) + validateDerivedBaseOutput(path, obj, "feat", report) +} +func validateDerivedBaseOutput(path string, obj map[string]any, datasetLabel string, report *ValidationReport) { if output, ok := obj["output"]; !ok { + return + } else if outputText, ok := output.(string); !ok || strings.TrimSpace(outputText) != nativeDatasetDefaultOutputName(filepath.Dir(path), nativeDatasetBase) { report.Diagnostics = append(report.Diagnostics, Diagnostic{ Severity: SeverityError, Path: path, - Message: "feat base.json must contain output set to feat.2da", - }) - } else if outputText, ok := output.(string); !ok || strings.TrimSpace(outputText) != "feat.2da" { - report.Diagnostics = append(report.Diagnostics, Diagnostic{ - Severity: SeverityError, - Path: path, - Message: "feat output must be feat.2da", + Message: fmt.Sprintf("%s output must be %s", datasetLabel, nativeDatasetDefaultOutputName(filepath.Dir(path), nativeDatasetBase)), }) } } diff --git a/internal/topdata/topdata_test.go b/internal/topdata/topdata_test.go index 3ef44cc..be5287f 100644 --- a/internal/topdata/topdata_test.go +++ b/internal/topdata/topdata_test.go @@ -46,6 +46,39 @@ func TestValidateProjectAcceptsInlineTLKSchema(t *testing.T) { } } +func TestValidateAndBuildDerivesFeatOutputWhenOmitted(t *testing.T) { + root := testProjectRoot(t) + mkdirAll(t, filepath.Join(root, "topdata", "data", "feat")) + writeFile(t, filepath.Join(root, "topdata", "base_dialog.json"), "{}\n") + writeFile(t, filepath.Join(root, "topdata", "data", "feat", "base.json"), `{ + "columns": ["LABEL", "FEAT", "DESCRIPTION"], + "rows": [ + { + "id": 0, + "key": "feat:test", + "LABEL": "FEAT_TEST", + "FEAT": {"tlk": {"key": "feat:test.name", "text": "Test Feat"}}, + "DESCRIPTION": {"tlk": {"key": "feat:test.description", "text": "Test Description"}} + } + ] +}`+"\n") + writeFile(t, filepath.Join(root, "topdata", "data", "feat", "lock.json"), `{"feat:test":0}`+"\n") + + proj := testProject(root) + report := ValidateProject(proj) + if report.HasErrors() { + t.Fatalf("expected omitted feat output to validate cleanly, got:\n%s", diagnosticsText(report.Diagnostics)) + } + + result, err := BuildNativeWithOptions(proj, NativeBuildOptions{BuildWiki: false}, nil) + if err != nil { + t.Fatalf("expected omitted feat output to build: %v", err) + } + if _, err := os.Stat(filepath.Join(result.Output2DADir, "feat.2da")); err != nil { + t.Fatalf("expected derived feat.2da output: %v", err) + } +} + func TestValidateProjectRejectsDuplicateJSONKeys(t *testing.T) { root := testProjectRoot(t) mkdirAll(t, filepath.Join(root, "topdata", "data", "repadjust")) @@ -860,6 +893,42 @@ func TestValidateProjectAcceptsCanonicalMasterfeatsContract(t *testing.T) { } } +func TestValidateAndBuildDerivesMasterfeatsOutputWhenOmitted(t *testing.T) { + root := testProjectRoot(t) + mkdirAll(t, filepath.Join(root, "topdata", "data", "masterfeats")) + writeFile(t, filepath.Join(root, "topdata", "base_dialog.json"), "{}\n") + writeFile(t, filepath.Join(root, "topdata", "data", "masterfeats", "base.json"), `{ + "columns": ["LABEL", "STRREF", "DESCRIPTION", "ICON"], + "rows": [ + { + "id": 0, + "key": "masterfeats:weaponfocus", + "LABEL": "WeaponFocus", + "STRREF": "6490", + "DESCRIPTION": "436", + "ICON": "ife_wepfoc" + } + ] +}`+"\n") + writeFile(t, filepath.Join(root, "topdata", "data", "masterfeats", "lock.json"), `{ + "masterfeats:weaponfocus": 0 +}`+"\n") + + proj := testProject(root) + report := ValidateProject(proj) + if report.HasErrors() { + t.Fatalf("expected omitted masterfeats output to validate cleanly, got:\n%s", diagnosticsText(report.Diagnostics)) + } + + result, err := BuildNativeWithOptions(proj, NativeBuildOptions{BuildWiki: false}, nil) + if err != nil { + t.Fatalf("expected omitted masterfeats output to build: %v", err) + } + if _, err := os.Stat(filepath.Join(result.Output2DADir, "masterfeats.2da")); err != nil { + t.Fatalf("expected derived masterfeats.2da output: %v", err) + } +} + func TestValidateProjectRejectsInvalidMasterfeatsContract(t *testing.T) { root := testProjectRoot(t) mkdirAll(t, filepath.Join(root, "topdata", "data", "masterfeats"))