docs: plan topdata JSON validation tightening
This commit is contained in:
@@ -0,0 +1,858 @@
|
|||||||
|
# Topdata JSON Validation Tightening Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use
|
||||||
|
> `superpowers:subagent-driven-development` (recommended) or
|
||||||
|
> `superpowers:executing-plans` to implement this plan task-by-task. Steps use
|
||||||
|
> checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Reject unsupported properties in established canonical topdata JSON
|
||||||
|
containers, add the non-empty `any_present` global-injection condition, and keep
|
||||||
|
standalone validation and direct native builds fail-closed and consistent.
|
||||||
|
|
||||||
|
**Architecture:** Add one focused `internal/topdata/json_contract.go` unit that
|
||||||
|
owns deterministic closed-key checks plus parsing and evaluation of global
|
||||||
|
injection condition groups. Existing validation in `topdata.go` uses that unit
|
||||||
|
to collect actionable diagnostics, while `native.go` uses the same parsed
|
||||||
|
conditions during ordered injection evaluation. Dataset payload rows continue
|
||||||
|
through the existing column-aware canonicalization paths and are not assigned a
|
||||||
|
global schema.
|
||||||
|
|
||||||
|
**Tech Stack:** Go 1.26, standard-library `encoding/json`, Go tests, Nix flakes,
|
||||||
|
Make, authored JSON in the sibling `sow-topdata` checkout.
|
||||||
|
|
||||||
|
## Global Constraints
|
||||||
|
|
||||||
|
- Work on a fresh non-`main` branch and open all changes as a pull request.
|
||||||
|
- Never commit or push secrets, `.sops` data, generated binaries, `.cache/`,
|
||||||
|
`generated/`, HAK/TLK artifacts, or result symlinks.
|
||||||
|
- Add no JSON Schema framework and no new validation dependency.
|
||||||
|
- Close only the canonical base/plain rows root, canonical module root,
|
||||||
|
`global.json` root, global injection, global condition, global default rule,
|
||||||
|
global default match object, and global default format object.
|
||||||
|
- Preserve specialized parser-owned JSON dialects outside those canonical
|
||||||
|
containers.
|
||||||
|
- Preserve dataset-specific columns and documented row controls inside `rows`,
|
||||||
|
`entries`, `overrides`, injection `row`, and default `values`.
|
||||||
|
- Preserve current-row, ordered-injection behavior: an earlier injection may
|
||||||
|
satisfy or block a later injection.
|
||||||
|
- Preserve empty-list behavior for `require_present` and `unless_present`;
|
||||||
|
reject an empty `any_present`.
|
||||||
|
- Do not add a compatibility alias for `when_present`.
|
||||||
|
- Use deterministic property ordering in diagnostics.
|
||||||
|
- Inventory evidence requires `compare_reference` to remain valid on
|
||||||
|
base/plain rows roots: `internal/topdata/native.go` reads it,
|
||||||
|
`internal/topdata/feat_migrate.go` emits it, and existing tests exercise it.
|
||||||
|
This is an established canonical property covered by the design's moderate
|
||||||
|
strictness rule even though the initial property table omitted it.
|
||||||
|
- The active `sow-topdata` branch
|
||||||
|
`fix-always-present-metamagic` already replaces the known `when_present` with
|
||||||
|
`require_present` in commit `ed139f5`; preserve that change and use the
|
||||||
|
checkout for integration validation rather than duplicating or rewriting it.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 1: Introduce the shared closed-object and condition contract
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
|
||||||
|
- Create: `internal/topdata/json_contract.go`
|
||||||
|
- Create: `internal/topdata/json_contract_test.go`
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
- Produces:
|
||||||
|
|
||||||
|
```go
|
||||||
|
type globalCondition struct {
|
||||||
|
Field string
|
||||||
|
ID string
|
||||||
|
}
|
||||||
|
|
||||||
|
type globalConditionGroups struct {
|
||||||
|
RequirePresent []globalCondition
|
||||||
|
AnyPresent []globalCondition
|
||||||
|
UnlessPresent []globalCondition
|
||||||
|
}
|
||||||
|
|
||||||
|
func unsupportedObjectKeys(obj map[string]any, supported ...string) []string
|
||||||
|
func unsupportedObjectKeysError(label string, obj map[string]any, supported ...string) error
|
||||||
|
func parseGlobalConditionGroups(injection map[string]any) (globalConditionGroups, error)
|
||||||
|
func globalConditionGroupsMatch(groups globalConditionGroups, rows []map[string]any) bool
|
||||||
|
```
|
||||||
|
|
||||||
|
- `unsupportedObjectKeys` returns unknown keys in lexical order.
|
||||||
|
- `parseGlobalConditionGroups` accepts absent groups, permits empty
|
||||||
|
`require_present` and `unless_present`, rejects empty `any_present`, validates
|
||||||
|
condition keys as exactly `field` and `id`, and returns the first deterministic
|
||||||
|
error.
|
||||||
|
- `globalConditionGroupsMatch` implements all/any/none semantics without
|
||||||
|
reparsing JSON.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Create the feature branch after checking it was not previously
|
||||||
|
merged and deleted**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git fetch origin
|
||||||
|
git branch --show-current
|
||||||
|
git ls-remote --heads origin feat/topdata-json-validation
|
||||||
|
git log --all --oneline --decorate --grep='topdata json validation'
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: current branch is not `main`. If
|
||||||
|
`feat/topdata-json-validation` has never been used, create it from the approved
|
||||||
|
design commit:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git switch -c feat/topdata-json-validation
|
||||||
|
```
|
||||||
|
|
||||||
|
If that exact branch name has prior remote history, choose a new descriptive
|
||||||
|
name and record it in the PR; do not reuse the old branch.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Re-run the canonical-format inventory before writing the
|
||||||
|
allowlists**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
for f in $(find ../sow-topdata/data -type f -name 'base.json' | sort); do
|
||||||
|
jq -r --arg f "${f#../sow-topdata/}" \
|
||||||
|
'[$f, (keys | sort | join(","))] | @tsv' "$f"
|
||||||
|
done
|
||||||
|
|
||||||
|
for f in $(find ../sow-topdata/data -type f -name 'global.json' | sort); do
|
||||||
|
jq -r --arg f "${f#../sow-topdata/}" \
|
||||||
|
'[$f, (keys | sort | join(","))] | @tsv' "$f"
|
||||||
|
done
|
||||||
|
|
||||||
|
find ../sow-topdata/data -type f -path '*/modules/*.json' -print0 |
|
||||||
|
while IFS= read -r -d '' f; do
|
||||||
|
jq -r --arg f "${f#../sow-topdata/}" \
|
||||||
|
'[$f, (keys | sort | join(","))] | @tsv' "$f"
|
||||||
|
done | sort
|
||||||
|
|
||||||
|
rg -n 'compare_reference' internal/topdata ../sow-topdata/data
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: active base roots use `columns`, `key`, `output`, and `rows`;
|
||||||
|
active modules use `columns`, `entries`, `overrides`, or the legitimate
|
||||||
|
`entries`+`overrides` combination; active globals use the approved global
|
||||||
|
keys. `sow-tools` additionally confirms established `compare_reference`
|
||||||
|
support.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Write failing unit tests for deterministic unsupported-key
|
||||||
|
diagnostics**
|
||||||
|
|
||||||
|
Add table-driven tests equivalent to:
|
||||||
|
|
||||||
|
```go
|
||||||
|
func TestUnsupportedObjectKeysAreSorted(t *testing.T) {
|
||||||
|
obj := map[string]any{
|
||||||
|
"row": map[string]any{},
|
||||||
|
"when_present": []any{},
|
||||||
|
"aaa": true,
|
||||||
|
}
|
||||||
|
|
||||||
|
got := unsupportedObjectKeys(
|
||||||
|
obj,
|
||||||
|
"row",
|
||||||
|
"require_present",
|
||||||
|
"any_present",
|
||||||
|
"unless_present",
|
||||||
|
)
|
||||||
|
want := []string{"aaa", "when_present"}
|
||||||
|
if !slices.Equal(got, want) {
|
||||||
|
t.Fatalf("unsupportedObjectKeys() = %v, want %v", got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestUnsupportedObjectKeysErrorNamesObjectAndContract(t *testing.T) {
|
||||||
|
err := unsupportedObjectKeysError(
|
||||||
|
"global injection 10",
|
||||||
|
map[string]any{"row": map[string]any{}, "when_present": []any{}},
|
||||||
|
"row",
|
||||||
|
"require_present",
|
||||||
|
"any_present",
|
||||||
|
"unless_present",
|
||||||
|
)
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected unsupported-property error")
|
||||||
|
}
|
||||||
|
for _, want := range []string{
|
||||||
|
"global injection 10",
|
||||||
|
`"when_present"`,
|
||||||
|
"row",
|
||||||
|
"require_present",
|
||||||
|
"any_present",
|
||||||
|
"unless_present",
|
||||||
|
} {
|
||||||
|
if !strings.Contains(err.Error(), want) {
|
||||||
|
t.Fatalf("error %q does not contain %q", err, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Write failing parser tests for all condition-group contracts**
|
||||||
|
|
||||||
|
Cover these exact cases in `json_contract_test.go`:
|
||||||
|
|
||||||
|
```go
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
injection map[string]any
|
||||||
|
want globalConditionGroups
|
||||||
|
wantError string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "all groups",
|
||||||
|
injection: map[string]any{
|
||||||
|
"require_present": []any{
|
||||||
|
map[string]any{"field": "FeatIndex", "id": "feat:required"},
|
||||||
|
},
|
||||||
|
"any_present": []any{
|
||||||
|
map[string]any{"field": "FeatIndex", "id": "feat:first"},
|
||||||
|
map[string]any{"field": "FeatIndex", "id": "feat:second"},
|
||||||
|
},
|
||||||
|
"unless_present": []any{
|
||||||
|
map[string]any{"field": "FeatIndex", "id": "feat:blocked"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
want: globalConditionGroups{
|
||||||
|
RequirePresent: []globalCondition{
|
||||||
|
{Field: "FeatIndex", ID: "feat:required"},
|
||||||
|
},
|
||||||
|
AnyPresent: []globalCondition{
|
||||||
|
{Field: "FeatIndex", ID: "feat:first"},
|
||||||
|
{Field: "FeatIndex", ID: "feat:second"},
|
||||||
|
},
|
||||||
|
UnlessPresent: []globalCondition{
|
||||||
|
{Field: "FeatIndex", ID: "feat:blocked"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "empty existing groups remain valid",
|
||||||
|
injection: map[string]any{
|
||||||
|
"require_present": []any{},
|
||||||
|
"unless_present": []any{},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "any present must be array",
|
||||||
|
injection: map[string]any{
|
||||||
|
"any_present": "feat:first",
|
||||||
|
},
|
||||||
|
wantError: "any_present must be an array",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "any present must not be empty",
|
||||||
|
injection: map[string]any{
|
||||||
|
"any_present": []any{},
|
||||||
|
},
|
||||||
|
wantError: "any_present must contain at least one condition",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "condition is closed",
|
||||||
|
injection: map[string]any{
|
||||||
|
"require_present": []any{
|
||||||
|
map[string]any{
|
||||||
|
"field": "FeatIndex",
|
||||||
|
"id": "feat:required",
|
||||||
|
"typo": true,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
wantError: `require_present[0] contains unsupported key "typo"`,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "field is required",
|
||||||
|
injection: map[string]any{
|
||||||
|
"require_present": []any{
|
||||||
|
map[string]any{"id": "feat:required"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
wantError: "require_present[0].field is required",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "id is required",
|
||||||
|
injection: map[string]any{
|
||||||
|
"require_present": []any{
|
||||||
|
map[string]any{"field": "FeatIndex"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
wantError: "require_present[0].id is required",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Write failing evaluator tests for all/any/none and group
|
||||||
|
conjunction**
|
||||||
|
|
||||||
|
Build rows with `FeatIndex` references for `feat:required`,
|
||||||
|
`feat:second`, and `feat:blocked`, then assert:
|
||||||
|
|
||||||
|
```go
|
||||||
|
if !globalConditionGroupsMatch(globalConditionGroups{
|
||||||
|
RequirePresent: []globalCondition{
|
||||||
|
{Field: "FeatIndex", ID: "feat:required"},
|
||||||
|
},
|
||||||
|
AnyPresent: []globalCondition{
|
||||||
|
{Field: "FeatIndex", ID: "feat:first"},
|
||||||
|
{Field: "FeatIndex", ID: "feat:second"},
|
||||||
|
},
|
||||||
|
}, rowsWithoutBlocked) {
|
||||||
|
t.Fatal("expected required + later any_present alternative to match")
|
||||||
|
}
|
||||||
|
|
||||||
|
if globalConditionGroupsMatch(globalConditionGroups{
|
||||||
|
AnyPresent: []globalCondition{
|
||||||
|
{Field: "FeatIndex", ID: "feat:missing"},
|
||||||
|
},
|
||||||
|
}, rowsWithoutBlocked) {
|
||||||
|
t.Fatal("expected missing any_present alternatives to reject")
|
||||||
|
}
|
||||||
|
|
||||||
|
if globalConditionGroupsMatch(globalConditionGroups{
|
||||||
|
UnlessPresent: []globalCondition{
|
||||||
|
{Field: "FeatIndex", ID: "feat:blocked"},
|
||||||
|
},
|
||||||
|
}, rowsWithBlocked) {
|
||||||
|
t.Fatal("expected unless_present match to reject")
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run the focused tests and confirm they fail**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
nix develop --command go test ./internal/topdata \
|
||||||
|
-run 'TestUnsupportedObjectKeys|TestParseGlobalConditionGroups|TestGlobalConditionGroupsMatch' \
|
||||||
|
-v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: build failure because the shared contract types and functions do
|
||||||
|
not exist.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Implement the minimal shared contract**
|
||||||
|
|
||||||
|
In `json_contract.go`, define fixed key sets and implement:
|
||||||
|
|
||||||
|
```go
|
||||||
|
var (
|
||||||
|
baseOrPlainRootKeys = []string{
|
||||||
|
"output", "key", "columns", "rows", "compare_reference",
|
||||||
|
}
|
||||||
|
canonicalModuleRootKeys = []string{
|
||||||
|
"output", "key", "columns", "entries", "overrides", "rows",
|
||||||
|
}
|
||||||
|
globalRootKeys = []string{
|
||||||
|
"columns", "entries", "overrides", "defaults", "position", "injections",
|
||||||
|
}
|
||||||
|
globalInjectionKeys = []string{
|
||||||
|
"row", "require_present", "any_present", "unless_present",
|
||||||
|
}
|
||||||
|
globalConditionKeys = []string{"field", "id"}
|
||||||
|
globalDefaultRuleKeys = []string{"match", "values"}
|
||||||
|
globalDefaultMatchKeys = []string{"source"}
|
||||||
|
globalDefaultFormatKeys = []string{"format"}
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
`unsupportedObjectKeysError` must report one deterministic error per call,
|
||||||
|
using the first lexically sorted unsupported key and a supported-key list in
|
||||||
|
declaration order. Implement condition parsing through one private helper:
|
||||||
|
|
||||||
|
```go
|
||||||
|
func parseGlobalConditionList(
|
||||||
|
name string,
|
||||||
|
raw any,
|
||||||
|
requireNonEmpty bool,
|
||||||
|
) ([]globalCondition, error)
|
||||||
|
```
|
||||||
|
|
||||||
|
It must reject non-arrays, reject non-object elements, run the closed-key
|
||||||
|
check before required-field checks, trim `field` and `id`, and enforce the
|
||||||
|
`any_present` non-empty rule.
|
||||||
|
|
||||||
|
`globalConditionGroupsMatch` must call the existing
|
||||||
|
`globalReferencePresent` helper and implement:
|
||||||
|
|
||||||
|
```text
|
||||||
|
require_present: every condition is found
|
||||||
|
any_present: at least one condition is found
|
||||||
|
unless_present: no condition is found
|
||||||
|
all groups: every present group passes
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 8: Run the focused and package tests**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
nix develop --command go test ./internal/topdata \
|
||||||
|
-run 'TestUnsupportedObjectKeys|TestParseGlobalConditionGroups|TestGlobalConditionGroupsMatch' \
|
||||||
|
-v
|
||||||
|
nix develop --command go test ./internal/topdata
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: pass.
|
||||||
|
|
||||||
|
- [ ] **Step 9: Commit the shared contract**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add internal/topdata/json_contract.go internal/topdata/json_contract_test.go
|
||||||
|
git commit -m "feat(topdata): define canonical json contracts"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 2: Close canonical structural objects during project validation
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
|
||||||
|
- Modify: `internal/topdata/topdata.go`
|
||||||
|
- Modify: `internal/topdata/json_contract_test.go`
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
- Consumes the key sets and condition parser from Task 1.
|
||||||
|
- `validateDataObject` selects exactly one root contract from file context:
|
||||||
|
base/plain rows root, canonical module root, or `global.json` root.
|
||||||
|
- A module validates every present supported container; it does not return
|
||||||
|
after the first of `entries`, `overrides`, `rows`, or `columns`.
|
||||||
|
- Specialized paths continue to return to their existing validators before
|
||||||
|
canonical root closure.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Add failing validation tests for unsupported canonical root
|
||||||
|
properties**
|
||||||
|
|
||||||
|
Add table-driven temporary-project cases covering:
|
||||||
|
- base root with `"typo": true`;
|
||||||
|
- loose plain rows root with `"typo": true`;
|
||||||
|
- module root with `"typo": true`;
|
||||||
|
- `global.json` root with `"typo": true`.
|
||||||
|
|
||||||
|
For every case, call `ValidateProject`, require errors, and assert only stable
|
||||||
|
semantic fragments: the file/object label, `typo`, and `unsupported`.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Add failing validation tests for closed nested global objects**
|
||||||
|
|
||||||
|
Add cases for:
|
||||||
|
|
||||||
|
```json
|
||||||
|
{"injections":[{"row":{},"when_present":[]}]}
|
||||||
|
{"injections":[{"row":{},"require_present":[{"field":"FeatIndex","id":"feat:x","typo":true}]}]}
|
||||||
|
{"defaults":[{"match":"all","values":{},"typo":true}]}
|
||||||
|
{"defaults":[{"match":{"source":"entries","typo":true},"values":{}}]}
|
||||||
|
{"defaults":[{"match":"all","values":{"ImpactScript":{"format":"ss_{id}","typo":true}}}]}
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected diagnostics name `global injection 0`, the condition label,
|
||||||
|
`global default 0`, `global default 0 match`, or
|
||||||
|
`global default 0 values.ImpactScript` respectively.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Add failing validation tests for `any_present` shape**
|
||||||
|
|
||||||
|
Test one accepted non-empty list and rejected string/empty-list cases. The
|
||||||
|
accepted case must use a declared dataset column in the injection row so this
|
||||||
|
also protects payload flexibility.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Add regression tests for canonical combinations and payload
|
||||||
|
flexibility**
|
||||||
|
|
||||||
|
Build one table-driven test that validates without errors:
|
||||||
|
- a columns-only module;
|
||||||
|
- an entries-only module;
|
||||||
|
- an overrides-only module;
|
||||||
|
- a module containing both entries and overrides;
|
||||||
|
- a rows module;
|
||||||
|
- a current base root;
|
||||||
|
- a loose plain rows root;
|
||||||
|
- a base/plain root with `compare_reference: false`;
|
||||||
|
- dataset columns plus `id`, `key`, `inherit`, `_tlk`, and `meta` in row-like
|
||||||
|
payloads where their existing contracts allow them;
|
||||||
|
- an injection row using a declared dataset column.
|
||||||
|
|
||||||
|
The combined entries+overrides case must contain an independently invalid
|
||||||
|
override value first, prove validation catches it, then fix only that value
|
||||||
|
and prove the module passes. This specifically prevents the current
|
||||||
|
first-container early return.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Run the focused tests and confirm failures**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
nix develop --command go test ./internal/topdata \
|
||||||
|
-run 'TestValidateProject(RejectsUnsupportedCanonicalJSON|RejectsUnsupportedGlobalJSON|ValidatesEveryCanonicalModuleContainer|AcceptsCanonicalJSONContracts|AcceptsAnyPresent|RejectsInvalidAnyPresent)' \
|
||||||
|
-v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: unknown keys are currently ignored, `any_present` is not validated,
|
||||||
|
and the combined module does not validate all containers.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Add a validation adapter for shared contract errors**
|
||||||
|
|
||||||
|
In `topdata.go`, add:
|
||||||
|
|
||||||
|
```go
|
||||||
|
func reportTopdataContractError(path string, err error, report *ValidationReport) {
|
||||||
|
if err == nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
report.Diagnostics = append(report.Diagnostics, Diagnostic{
|
||||||
|
Severity: SeverityError,
|
||||||
|
Path: path,
|
||||||
|
Message: err.Error(),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Use `unsupportedObjectKeysError` for canonical roots and nested structural
|
||||||
|
objects. Keep row payload validation in existing `validateRowCollection`,
|
||||||
|
`validateEntriesFile`, and `validateOverridesFile` paths.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Refactor canonical module dispatch to validate all containers**
|
||||||
|
|
||||||
|
After specialized-path checks, classify module files by directory context.
|
||||||
|
For recognized canonical modules:
|
||||||
|
1. check `canonicalModuleRootKeys`;
|
||||||
|
2. call `validateColumnsFile` when `columns` is present;
|
||||||
|
3. call `validateEntriesFile` when `entries` is present;
|
||||||
|
4. call `validateOverridesFile` when `overrides` is present;
|
||||||
|
5. call `validateRowsFile(path, obj, report, false)` when `rows` is present;
|
||||||
|
6. emit the existing warning only when none of those containers is present.
|
||||||
|
|
||||||
|
Avoid duplicate `validateColumnsFile` calls by removing the nested column
|
||||||
|
calls from this module dispatch path or by adding a caller-controlled flag;
|
||||||
|
retain current behavior for callers outside canonical module dispatch.
|
||||||
|
|
||||||
|
- [ ] **Step 8: Close global roots, injections, defaults, matches, and format
|
||||||
|
objects**
|
||||||
|
- Run the global-root key check before validating present containers.
|
||||||
|
- Run the injection key check before validating `row` and condition groups.
|
||||||
|
- Replace calls to `validateGlobalConditionList` with
|
||||||
|
`parseGlobalConditionGroups`; report its error through
|
||||||
|
`reportTopdataContractError`.
|
||||||
|
- Run default-rule and match-object key checks before existing type/value
|
||||||
|
checks.
|
||||||
|
- Treat an object containing `format` as a format object, reject every key
|
||||||
|
other than `format`, and preserve non-format object literals as payload
|
||||||
|
values.
|
||||||
|
|
||||||
|
- [ ] **Step 9: Run focused and full package tests**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
nix develop --command go test ./internal/topdata \
|
||||||
|
-run 'TestValidateProject(RejectsUnsupportedCanonicalJSON|RejectsUnsupportedGlobalJSON|ValidatesEveryCanonicalModuleContainer|AcceptsCanonicalJSONContracts|AcceptsAnyPresent|RejectsInvalidAnyPresent)' \
|
||||||
|
-v
|
||||||
|
nix develop --command go test ./internal/topdata
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: pass.
|
||||||
|
|
||||||
|
- [ ] **Step 10: Commit validation tightening**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add internal/topdata/topdata.go internal/topdata/json_contract_test.go
|
||||||
|
git commit -m "feat(topdata): reject unsupported canonical json properties"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 3: Use shared condition parsing and `any_present` in native builds
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
|
||||||
|
- Modify: `internal/topdata/native.go`
|
||||||
|
- Modify: `internal/topdata/json_contract_test.go`
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
- Consumes `parseGlobalConditionGroups` and
|
||||||
|
`globalConditionGroupsMatch` from Task 1.
|
||||||
|
- Removes the old boolean-mode
|
||||||
|
`globalConditionListMatches(name, raw, rows, required)` path.
|
||||||
|
- Direct `BuildNativeWithOptions` calls fail before output mutation when
|
||||||
|
canonical control syntax is invalid because the existing build entry point
|
||||||
|
runs `ValidateProject` first and native evaluation uses the same parser.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Add failing native-build tests for `any_present` alternatives**
|
||||||
|
|
||||||
|
Use one compact plain-dataset fixture with:
|
||||||
|
- an authored row satisfying the first alternative;
|
||||||
|
- a second subtest satisfying only the later alternative;
|
||||||
|
- a third subtest satisfying neither alternative.
|
||||||
|
|
||||||
|
Assert the injected row exists in the first two outputs and is absent in the
|
||||||
|
third. Inspect the built 2DA content rather than internal slices.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Add failing native-build tests for condition-group
|
||||||
|
conjunction**
|
||||||
|
|
||||||
|
Add subtests proving:
|
||||||
|
- `require_present` + `any_present` injects only when both pass;
|
||||||
|
- `any_present` + `unless_present` injects only when both pass;
|
||||||
|
- existing `require_present` all semantics remain unchanged;
|
||||||
|
- existing `unless_present` none semantics remain unchanged.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Add a failing ordered-injection test**
|
||||||
|
|
||||||
|
Author injection 0 so it adds the row referenced by injection 1's
|
||||||
|
`any_present`. Assert injection 1 applies. This protects evaluation against
|
||||||
|
the same current-row view and ordered mutation used today.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Add direct-build fail-closed tests**
|
||||||
|
|
||||||
|
Call `BuildNativeWithOptions` without a prior explicit `ValidateProject` call
|
||||||
|
for fixtures containing:
|
||||||
|
- `when_present` on an injection;
|
||||||
|
- an unknown condition key;
|
||||||
|
- an empty `any_present`;
|
||||||
|
- an unknown canonical root key.
|
||||||
|
|
||||||
|
Require an error containing the stable object/property fragments and assert
|
||||||
|
the expected output 2DA does not exist.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Run focused tests and confirm failures**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
nix develop --command go test ./internal/topdata \
|
||||||
|
-run 'TestBuildNative(AnyPresent|CombinesGlobalConditionGroups|UsesCurrentRowsForAnyPresent|RejectsUnsupportedGlobalControlSyntax|RejectsUnsupportedCanonicalRoot)' \
|
||||||
|
-v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `any_present` is ignored by current native evaluation and the new
|
||||||
|
parser/evaluator is not wired.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Replace native condition evaluation with the shared contract**
|
||||||
|
|
||||||
|
Replace `globalInjectionConditionsMatch` and
|
||||||
|
`globalConditionListMatches` with:
|
||||||
|
|
||||||
|
```go
|
||||||
|
func globalInjectionConditionsMatch(
|
||||||
|
injection map[string]any,
|
||||||
|
rows []map[string]any,
|
||||||
|
) (bool, error) {
|
||||||
|
groups, err := parseGlobalConditionGroups(injection)
|
||||||
|
if err != nil {
|
||||||
|
return false, err
|
||||||
|
}
|
||||||
|
return globalConditionGroupsMatch(groups, rows), nil
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Keep `applyPlainDatasetGlobalInjections` calling this function immediately
|
||||||
|
before deduplication/canonicalization, using `currentRows()` for each
|
||||||
|
injection.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Run focused, package, and repository tests**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
nix develop --command go test ./internal/topdata \
|
||||||
|
-run 'TestBuildNative(AnyPresent|CombinesGlobalConditionGroups|UsesCurrentRowsForAnyPresent|RejectsUnsupportedGlobalControlSyntax|RejectsUnsupportedCanonicalRoot)' \
|
||||||
|
-v
|
||||||
|
nix develop --command go test ./internal/topdata
|
||||||
|
nix develop --command go test ./...
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: pass.
|
||||||
|
|
||||||
|
- [ ] **Step 8: Commit native `any_present` support**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add internal/topdata/native.go internal/topdata/json_contract_test.go
|
||||||
|
git commit -m "feat(topdata): add any-present injection conditions"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 4: Document the global injection grammar
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
|
||||||
|
- Modify: `internal/topdata/CLASS_FEAT_GLOBAL_INJECTS_CONTRACT.md`
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
- Documents the exact injection keys and all/any/none condition semantics.
|
||||||
|
- States that all present groups must pass and that conditions observe current
|
||||||
|
rows in injection order.
|
||||||
|
- States that `any_present` must be a non-empty array and `when_present` is
|
||||||
|
invalid.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Add a documentation contract check**
|
||||||
|
|
||||||
|
Run this before editing:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
rg -n 'any_present|when_present|all.*any.*none|require_present.*unless_present' \
|
||||||
|
internal/topdata/CLASS_FEAT_GLOBAL_INJECTS_CONTRACT.md
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: no complete description of `any_present` and group conjunction.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Update the manifest shape and semantics**
|
||||||
|
|
||||||
|
Add an example containing all three groups:
|
||||||
|
|
||||||
|
```json
|
||||||
|
{
|
||||||
|
"row": {
|
||||||
|
"FeatIndex": { "id": "feat:example" }
|
||||||
|
},
|
||||||
|
"require_present": [{ "field": "FeatIndex", "id": "feat:required" }],
|
||||||
|
"any_present": [
|
||||||
|
{ "field": "FeatIndex", "id": "masterfeats:metamagic" },
|
||||||
|
{ "field": "FeatIndex", "id": "masterfeats:combat" }
|
||||||
|
],
|
||||||
|
"unless_present": [{ "field": "FeatIndex", "id": "feat:example" }]
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Document:
|
||||||
|
- `require_present`: all listed references must exist;
|
||||||
|
- `any_present`: at least one listed reference must exist and the list cannot
|
||||||
|
be empty;
|
||||||
|
- `unless_present`: none of the listed references may exist;
|
||||||
|
- every present group must pass;
|
||||||
|
- earlier injections affect later conditions;
|
||||||
|
- supported injection keys are `row`, `require_present`, `any_present`, and
|
||||||
|
`unless_present`;
|
||||||
|
- `when_present` is rejected rather than aliased.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Verify the documentation contains the complete contract**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
rg -n 'require_present|any_present|unless_present|when_present|earlier injection|every present group' \
|
||||||
|
internal/topdata/CLASS_FEAT_GLOBAL_INJECTS_CONTRACT.md
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: each semantic point appears in the contract.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Commit documentation**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add internal/topdata/CLASS_FEAT_GLOBAL_INJECTS_CONTRACT.md
|
||||||
|
git commit -m "docs(topdata): define global injection conditions"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 5: Validate the active topdata checkout and complete repository checks
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
|
||||||
|
- No authored files should change in `sow-tools`.
|
||||||
|
- Preserve existing sibling change:
|
||||||
|
`../sow-topdata/data/classes/feats/global.json`.
|
||||||
|
- Do not retain: `bin/`, `.cache/`, `generated/`, `result`, HAK/TLK files, or
|
||||||
|
temporary Crucible binaries.
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
|
||||||
|
- The built Crucible validates the active `sow-topdata` checkout.
|
||||||
|
- The active checkout's single-condition correction uses `require_present`;
|
||||||
|
`any_present` remains available for future multi-alternative authoring.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Format and run the full `sow-tools` checks**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
nix develop --command gofmt -w \
|
||||||
|
internal/topdata/json_contract.go \
|
||||||
|
internal/topdata/json_contract_test.go \
|
||||||
|
internal/topdata/topdata.go \
|
||||||
|
internal/topdata/native.go
|
||||||
|
nix develop --command make check
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: vet, Go tests, shellcheck, and yamllint pass.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Build a temporary Crucible binary without tracking it**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
nix develop --command go build \
|
||||||
|
-o /tmp/crucible-topdata-json-validation \
|
||||||
|
./cmd/crucible
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `/tmp/crucible-topdata-json-validation` exists outside the
|
||||||
|
repository.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Confirm the active consumer syntax and repository state**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git -C ../sow-topdata status --short --branch
|
||||||
|
rg -n 'when_present|require_present|any_present|unless_present' \
|
||||||
|
../sow-topdata/data/classes/feats/global.json
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: the existing consumer branch contains `require_present` and no
|
||||||
|
`when_present`. Do not alter unrelated consumer work.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Validate and build active `sow-topdata` with the changed
|
||||||
|
Crucible**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
(
|
||||||
|
cd ../sow-topdata
|
||||||
|
nix develop --command /tmp/crucible-topdata-json-validation topdata validate
|
||||||
|
nix develop --command /tmp/crucible-topdata-json-validation topdata build
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: validation and build pass, global injections execute, and generated
|
||||||
|
outputs remain ignored.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Verify no generated files became tracked**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git status --short
|
||||||
|
git -C ../sow-topdata status --short
|
||||||
|
git ls-files --error-unmatch bin generated .cache result 2>/dev/null
|
||||||
|
git -C ../sow-topdata ls-files --error-unmatch generated .cache result 2>/dev/null
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: the first two commands show only intentional source/doc changes;
|
||||||
|
the `git ls-files --error-unmatch` commands find no newly tracked generated
|
||||||
|
output. Remove only untracked artifacts created by this plan.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run final semantic searches and diff checks**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
rg -n 'when_present' internal/topdata --glob '!docs/superpowers/**'
|
||||||
|
rg -n 'any_present' \
|
||||||
|
internal/topdata/json_contract.go \
|
||||||
|
internal/topdata/json_contract_test.go \
|
||||||
|
internal/topdata/CLASS_FEAT_GLOBAL_INJECTS_CONTRACT.md
|
||||||
|
git diff --check
|
||||||
|
git status --short --branch
|
||||||
|
git log --oneline origin/main..HEAD
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `when_present` appears only in rejection tests/documentation;
|
||||||
|
implementation, tests, and contract document `any_present`; diff check is
|
||||||
|
clean.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Remove the temporary binary**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
rm -f /tmp/crucible-topdata-json-validation
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 8: Push the feature branch and open the required pull request**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git push -u origin HEAD
|
||||||
|
tea pr create \
|
||||||
|
--title "feat: tighten topdata JSON validation" \
|
||||||
|
--description $'## Summary\n- reject unsupported canonical topdata JSON properties\n- add non-empty any_present global injection conditions\n- keep validation and direct builds on one shared contract\n\n## Verification\n- nix develop --command make check\n- active sow-topdata validate and build with the changed Crucible'
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: a PR targeting `main`; no production workflow or release tag is
|
||||||
|
triggered.
|
||||||
|
|
||||||
|
- [ ] **Step 9: Record the handoff**
|
||||||
|
|
||||||
|
Report:
|
||||||
|
- the `sow-tools` branch and PR URL;
|
||||||
|
- the existing `sow-topdata` consumer branch/commit used for integration;
|
||||||
|
- exact focused and full verification commands with outcomes;
|
||||||
|
- confirmation that no generated files were tracked;
|
||||||
|
- any diagnostics wording intentionally asserted as public semantic
|
||||||
|
contract.
|
||||||
Reference in New Issue
Block a user