diff --git a/docs/superpowers/specs/2026-06-25-topdata-json-validation-design.md b/docs/superpowers/specs/2026-06-25-topdata-json-validation-design.md index 756ea3e..bd203cc 100644 --- a/docs/superpowers/specs/2026-06-25-topdata-json-validation-design.md +++ b/docs/superpowers/specs/2026-06-25-topdata-json-validation-design.md @@ -1,7 +1,7 @@ # Topdata JSON Validation Tightening Design **Date:** 2026-06-25 -**Status:** Approved design; implementation pending +**Status:** Implemented and reviewed **Scope:** `sow-tools` ## Problem diff --git a/internal/topdata/json_contract.go b/internal/topdata/json_contract.go index 7da1222..3f9df1e 100644 --- a/internal/topdata/json_contract.go +++ b/internal/topdata/json_contract.go @@ -1,6 +1,7 @@ package topdata import ( + "errors" "fmt" "slices" "strings" @@ -63,36 +64,65 @@ func unsupportedObjectKeysError(label string, obj map[string]any, supported ...s return fmt.Errorf("%s contains unsupported key %q (supported keys: %s)", label, unsupported[0], strings.Join(supported, ", ")) } -func parseGlobalConditionGroups(injection map[string]any) (globalConditionGroups, error) { +type globalConditionParseOptions struct { + Label string + Report func(error) +} + +func parseGlobalConditionGroups(injection map[string]any, options ...globalConditionParseOptions) (globalConditionGroups, error) { if injection == nil { return globalConditionGroups{}, nil } - if err := unsupportedObjectKeysError("global injection", injection, globalInjectionKeys...); err != nil { - return globalConditionGroups{}, err + + label := "global injection" + var report func(error) + if len(options) > 0 { + if strings.TrimSpace(options[0].Label) != "" { + label = options[0].Label + } + report = options[0].Report + } + + var errs []error + addError := func(err error) { + if err == nil { + return + } + errs = append(errs, err) + if report != nil { + report(err) + } + } + if err := unsupportedObjectKeysError(label, injection, globalInjectionKeys...); err != nil { + addError(err) } var groups globalConditionGroups - var err error - if raw, ok := injection["require_present"]; ok { - groups.RequirePresent, err = parseGlobalConditionList("require_present", raw, false) + parsed, err := parseGlobalConditionList(label+" require_present", raw, false) if err != nil { - return globalConditionGroups{}, err + addError(err) + } else { + groups.RequirePresent = parsed } } if raw, ok := injection["any_present"]; ok { - groups.AnyPresent, err = parseGlobalConditionList("any_present", raw, true) + parsed, err := parseGlobalConditionList(label+" any_present", raw, true) if err != nil { - return globalConditionGroups{}, err + addError(err) + } else { + groups.AnyPresent = parsed } } if raw, ok := injection["unless_present"]; ok { - groups.UnlessPresent, err = parseGlobalConditionList("unless_present", raw, false) + parsed, err := parseGlobalConditionList(label+" unless_present", raw, false) if err != nil { - return globalConditionGroups{}, err + addError(err) + } else { + groups.UnlessPresent = parsed } } - return groups, nil + return groups, errors.Join(errs...) } func parseGlobalConditionList(name string, raw any, requireNonEmpty bool) ([]globalCondition, error) { diff --git a/internal/topdata/json_contract_test.go b/internal/topdata/json_contract_test.go index 25cb532..609076e 100644 --- a/internal/topdata/json_contract_test.go +++ b/internal/topdata/json_contract_test.go @@ -1,6 +1,7 @@ package topdata import ( + "bytes" "errors" "os" "path/filepath" @@ -309,6 +310,59 @@ func TestValidateProjectRejectsUnsupportedCanonicalJSON(t *testing.T) { } } +func TestValidateProjectRejectsUnsupportedInvariantBaseRoots(t *testing.T) { + tests := []struct { + name string + dataset string + base string + lock string + }{ + { + name: "feat", + dataset: "feat", + base: `{ + "output": "feat.2da", + "columns": ["LABEL", "FEAT", "DESCRIPTION"], + "rows": [ + {"id": 0, "key": "feat:test", "LABEL": "FEAT_TEST", "FEAT": "100", "DESCRIPTION": "200"} + ], + "typo": true +}` + "\n", + lock: `{"feat:test":0}` + "\n", + }, + { + name: "masterfeats", + dataset: "masterfeats", + base: `{ + "output": "masterfeats.2da", + "columns": ["LABEL", "STRREF", "DESCRIPTION"], + "rows": [ + {"id": 0, "key": "masterfeats:test", "LABEL": "Test", "STRREF": "100", "DESCRIPTION": "200"} + ], + "typo": true +}` + "\n", + lock: `{"masterfeats:test":0}` + "\n", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + root := testProjectRoot(t) + writeCanonicalContractScaffold(t, root) + datasetDir := filepath.Join(root, "topdata", "data", tc.dataset) + mkdirAll(t, datasetDir) + writeFile(t, filepath.Join(datasetDir, "base.json"), tc.base) + writeFile(t, filepath.Join(datasetDir, "lock.json"), tc.lock) + + report := ValidateProject(testProject(root)) + if !report.HasErrors() { + t.Fatalf("expected unsupported invariant base-root key to fail validation, got:\n%s", diagnosticsText(report.Diagnostics)) + } + assertDiagnosticTextContainsAll(t, report.Diagnostics, "base.json root", "typo", "unsupported") + }) + } +} + func TestValidateProjectRejectsUnsupportedGlobalJSON(t *testing.T) { tests := []struct { name string @@ -339,7 +393,7 @@ func TestValidateProjectRejectsUnsupportedGlobalJSON(t *testing.T) { } ] }` + "\n", - fragments: []string{"require_present[0]", "typo", "unsupported"}, + fragments: []string{"global injection 0 require_present[0]", "typo", "unsupported"}, }, { name: "default rule is closed", @@ -483,7 +537,7 @@ func TestValidateProjectReportsUnsupportedGlobalInjectionKeysAndInvalidAnyPresen } assertDiagnosticTextContainsAll(t, report.Diagnostics, `global injection 0 contains unsupported key "when_present"`, - "any_present must contain at least one condition", + "global injection 0 any_present must contain at least one condition", ) } @@ -1133,6 +1187,80 @@ func TestBuildNativeRejectsUnsupportedCanonicalRoot(t *testing.T) { } } +func TestBuildNativeRejectsUnsupportedInvariantBaseRootsWithoutChangingOutput(t *testing.T) { + tests := []struct { + name string + dataset string + outputName string + base string + lock string + }{ + { + name: "feat", + dataset: "feat", + outputName: "feat.2da", + base: `{ + "output": "feat.2da", + "columns": ["LABEL", "FEAT", "DESCRIPTION"], + "rows": [ + {"id": 0, "key": "feat:test", "LABEL": "FEAT_TEST", "FEAT": "100", "DESCRIPTION": "200"} + ], + "typo": true +}` + "\n", + lock: `{"feat:test":0}` + "\n", + }, + { + name: "masterfeats", + dataset: "masterfeats", + outputName: "masterfeats.2da", + base: `{ + "output": "masterfeats.2da", + "columns": ["LABEL", "STRREF", "DESCRIPTION"], + "rows": [ + {"id": 0, "key": "masterfeats:test", "LABEL": "Test", "STRREF": "100", "DESCRIPTION": "200"} + ], + "typo": true +}` + "\n", + lock: `{"masterfeats:test":0}` + "\n", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + root := testProjectRoot(t) + writeCanonicalContractScaffold(t, root) + datasetDir := filepath.Join(root, "topdata", "data", tc.dataset) + mkdirAll(t, datasetDir) + writeFile(t, filepath.Join(datasetDir, "base.json"), tc.base) + writeFile(t, filepath.Join(datasetDir, "lock.json"), tc.lock) + + proj := testProject(root) + proj.Config.TopData.ReferenceBuilder = "" + outputPath := filepath.Join(compiled2DAOutputDir(proj), tc.outputName) + mkdirAll(t, filepath.Dir(outputPath)) + sentinel := []byte("pre-existing output sentinel\n") + writeBytes(t, outputPath, sentinel) + + _, err := BuildNativeWithOptions(proj, NativeBuildOptions{BuildWiki: false}, nil) + if err == nil { + t.Fatal("expected BuildNativeWithOptions to fail") + } + for _, fragment := range []string{"base.json root", "typo", "unsupported"} { + if !strings.Contains(err.Error(), fragment) { + t.Fatalf("error %q does not contain %q", err, fragment) + } + } + got, readErr := os.ReadFile(outputPath) + if readErr != nil { + t.Fatalf("read preserved output: %v", readErr) + } + if !bytes.Equal(got, sentinel) { + t.Fatalf("output changed after invalid build error:\ngot: %q\nwant: %q", got, sentinel) + } + }) + } +} + func tableContainsCellValue(table twoDATable, column, want string) bool { for _, row := range table.rows { if row[column] == want { diff --git a/internal/topdata/topdata.go b/internal/topdata/topdata.go index 8e96b39..2300373 100644 --- a/internal/topdata/topdata.go +++ b/internal/topdata/topdata.go @@ -532,6 +532,7 @@ func datasetInvariantForPath(dataPath topdataDataPath) (datasetValidationInvaria } func validateDatasetInvariantBaseFile(path string, obj map[string]any, invariant datasetValidationInvariant, report *ValidationReport) { + reportTopdataContractError(path, unsupportedObjectKeysError("base.json root", obj, baseOrPlainRootKeys...), report) validateRowsFile(path, obj, report, true) validateDerivedBaseOutput(path, obj, invariant.Name, report) validateRequiredColumns(path, obj, invariant, report) @@ -990,32 +991,6 @@ func validateOverridesFileWithColumns(path string, obj map[string]any, report *V } } -func parseGlobalConditionGroupsWithoutKeyCheck(injection map[string]any) (globalConditionGroups, error) { - var groups globalConditionGroups - if raw, ok := injection["require_present"]; ok { - parsed, err := parseGlobalConditionList("require_present", raw, false) - if err != nil { - return globalConditionGroups{}, err - } - groups.RequirePresent = parsed - } - if raw, ok := injection["any_present"]; ok { - parsed, err := parseGlobalConditionList("any_present", raw, true) - if err != nil { - return globalConditionGroups{}, err - } - groups.AnyPresent = parsed - } - if raw, ok := injection["unless_present"]; ok { - parsed, err := parseGlobalConditionList("unless_present", raw, false) - if err != nil { - return globalConditionGroups{}, err - } - groups.UnlessPresent = parsed - } - return groups, nil -} - func validateGlobalJSONFile(path string, obj map[string]any, report *ValidationReport) { reportTopdataContractError(path, unsupportedObjectKeysError("global.json root", obj, globalRootKeys...), report) if _, ok := obj["columns"]; ok { @@ -1074,8 +1049,6 @@ func validateGlobalJSONFile(path string, obj map[string]any, report *ValidationR }) continue } - err := unsupportedObjectKeysError(fmt.Sprintf("global injection %d", index), injection, globalInjectionKeys...) - reportTopdataContractError(path, err, report) row, ok := injection["row"].(map[string]any) if !ok { report.Diagnostics = append(report.Diagnostics, Diagnostic{ @@ -1086,8 +1059,12 @@ func validateGlobalJSONFile(path string, obj map[string]any, report *ValidationR } else { rows = append(rows, row) } - _, conditionErr := parseGlobalConditionGroupsWithoutKeyCheck(injection) - reportTopdataContractError(path, conditionErr, report) + _, _ = parseGlobalConditionGroups(injection, globalConditionParseOptions{ + Label: fmt.Sprintf("global injection %d", index), + Report: func(err error) { + reportTopdataContractError(path, err, report) + }, + }) } validateRowCollection(path, obj, rows, report) } @@ -1269,46 +1246,6 @@ func validateGlobalDefaultValue(path string, index int, field string, value any, } } -func validateGlobalConditionList(path, label string, raw any, report *ValidationReport) { - if raw == nil { - return - } - conditions, ok := raw.([]any) - if !ok { - report.Diagnostics = append(report.Diagnostics, Diagnostic{ - Severity: SeverityError, - Path: path, - Message: fmt.Sprintf("%s must be a JSON array", label), - }) - return - } - for index, rawCondition := range conditions { - condition, ok := rawCondition.(map[string]any) - if !ok { - report.Diagnostics = append(report.Diagnostics, Diagnostic{ - Severity: SeverityError, - Path: path, - Message: fmt.Sprintf("%s[%d] must be an object", label, index), - }) - continue - } - if field, ok := condition["field"].(string); !ok || strings.TrimSpace(field) == "" { - report.Diagnostics = append(report.Diagnostics, Diagnostic{ - Severity: SeverityError, - Path: path, - Message: fmt.Sprintf("%s[%d].field is required", label, index), - }) - } - if id, ok := condition["id"].(string); !ok || strings.TrimSpace(id) == "" { - report.Diagnostics = append(report.Diagnostics, Diagnostic{ - Severity: SeverityError, - Path: path, - Message: fmt.Sprintf("%s[%d].id is required", label, index), - }) - } - } -} - func validateStateObject(path string, obj map[string]any, report *ValidationReport) { entries, ok := obj["entries"].(map[string]any) if !ok {