From 0a75af512200f073ea3b0b655fed2bdaa11be0f6 Mon Sep 17 00:00:00 2001 From: Meketreve <34199654+meketreve@users.noreply.github.com> Date: Sun, 16 Aug 2026 03:34:29 -0300 Subject: [PATCH] Make the content validation checklist executable Asked for on Discord on 2026-08-15: if the project has tests now and wants them added alongside new content, there should be instructions telling Claude how to do that. The instructions turned out to be the smaller half. Section 26 of the content guide is already a list of invariants -- unique names, referenced fields exist, all recipe components exist, tech tree parents exist, coordinates do not overlap, extensions resolve, robot parts exist -- and every one of them is a statement a query can settle. None had ever been run as code. ContentInvariantTests turns ten of them into read-only queries against the real perpetuumsa, in the shape Schema/StoredProcedureConformanceTests already uses. All ten hold on the shipped P36.8 database, so this starts green rather than red. One needed measuring rather than assuming. techtree carries 21 rows whose parentdefinition is 0, which is the root-node marker and not a broken reference: entitydefaults.definition is IDENTITY(1,1) and its lowest live value is 1, so 0 can never name a real definition. Writing that exclusion in blind would have hidden a genuine dangling parent; leaving it out reports 21 healthy roots as damage. An invariant passes by counting zero, so a query that can never match passes for the wrong reason and stays green through any amount of broken content -- a NULL in the referencing column is the easy way for that to happen. A second test runs the same shape against a set built in the query, holding one dangling reference and one NULL, and asserts it finds exactly the dangling one. It was observed failing (expected 1, actual 0) with the dangling row removed. The instructions then go where an agent actually reads them: - content guide section 26 gains the command, the warning that a skipped run is not a pass, and an explicit list of what the tests do not cover - TESTING.md gains a sixth rule for content, which is data rather than code and so has nothing to say to tier 2 - CLAUDE.md's Game Content Creation rules gain the run and a requirement to report what the invariants did not check All three say the same thing in the same words: a green run means the content hangs together, not that the content is good. Balance, cost, tiering and sibling-matched module flags are judgements no query makes, and a checklist that implied otherwise would be worse than no checklist. Verified: integration tier 10/10 with PERPETUUM_GAMEROOT set, and the detection test observed failing before it passed. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 3 +- docs/codebase/TESTING.md | 15 +- docs/content/claude_game_content_guide.md | 22 +++ .../Content/ContentInvariantTests.cs | 165 ++++++++++++++++++ 4 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 src/Perpetuum.Tests.Integration/Content/ContentInvariantTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 36dbd9a..c8e05f8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -147,7 +147,8 @@ Claude MUST: - Use naming conventions from the guide (section 3): `def_`, `_pr`, `_cprg`, `effect_`, `cf_` prefixes. - Use idempotent SQL patterns: `MERGE`, `IF NOT EXISTS`, or `DELETE + INSERT` as appropriate per table. - Generate full-chain content when possible — avoid partial generation. -- Run the validation checklist (section 26) before declaring content complete. +- Run the validation checklist (section 26) before declaring content complete. Most of it is executable: apply the content to a local database and run `ContentInvariantTests` in `Perpetuum.Tests.Integration`. It skips when `PERPETUUM_GAMEROOT` is unset, so confirm it ran rather than assuming a pass. +- Report what the invariants said, and state separately what they do not cover — balance, cost, tiering and sibling-matched flags are judgements no query makes. - Ask the user for existing database values when dynamic resolution requires live data not available in docs. Claude MUST NOT: diff --git a/docs/codebase/TESTING.md b/docs/codebase/TESTING.md index 40d0456..eb4535d 100644 --- a/docs/codebase/TESTING.md +++ b/docs/codebase/TESTING.md @@ -11,7 +11,7 @@ coverage map below states what is covered and what is not. |------|---------|-------|-------| | 1 — smoke | `tools/smoke-test.ps1` | 1 end-to-end run | A configured `GameRoot` and a live database | | 2 — unit | `src/Perpetuum.Tests` | 58 tests | Nothing. Runs anywhere the solution builds | -| 3 — integration | `src/Perpetuum.Tests.Integration` | 8 tests | A configured `GameRoot` and a live database | +| 3 — integration | `src/Perpetuum.Tests.Integration` | 10 tests | A configured `GameRoot` and a live database | Tier 2 is the tier that runs in CI. Tiers 1 and 3 run on a developer machine that already has the standard server environment, and skip rather than fail when it is absent. @@ -205,6 +205,19 @@ planned order of attack. 5. Production code is not restructured to make a test possible. The four seams above have been enough so far; if a test genuinely cannot be written without a new seam, that is a discussion to have in the pull request, not a refactor to slip in. +6. **Game content is tested differently, because it is data rather than code.** New items, robots, + effects, modules or tech tree nodes are rows, so tier 2 has nothing to say about them — a faked + data layer cannot tell you whether a recipe names a component that exists. They belong in + `src/Perpetuum.Tests.Integration/Content/ContentInvariantTests.cs`, which turns the validation + checklist in `docs/content/claude_game_content_guide.md` section 26 into queries. Adding a new + kind of content usually means adding an invariant there rather than a test per item: the + invariant holds for every row of that kind, including the ones nobody has written yet. + +An invariant test passes by counting zero, which means a query that can never match passes for the +wrong reason and stays green through any amount of broken content. Write the new invariant, run it, +and confirm it reports a violation you know is there before trusting a zero. `ContentInvariantTests` +carries a test that does this for the shape the others use, including the NULL case that silently +breaks it. --- diff --git a/docs/content/claude_game_content_guide.md b/docs/content/claude_game_content_guide.md index 7e07e8a..94a2dea 100644 --- a/docs/content/claude_game_content_guide.md +++ b/docs/content/claude_game_content_guide.md @@ -1086,6 +1086,28 @@ Support: # 26. Validation Checklist +**Most of this checklist is now executable.** Apply your content to a local database, then run: + +```bash +dotnet test src/Perpetuum.Tests.Integration/Perpetuum.Tests.Integration.csproj -c Release -p:Platform=x64 --filter "FullyQualifiedName~ContentInvariantTests" +``` + +`ContentInvariantTests` checks the structural half of the list below against the real database: +recipe definitions and components resolve, no definition is a component of itself, tech tree parents +and children exist, no two nodes share a coordinate in a group, enabler extensions resolve, robot +template relations resolve on both sides, and no two definitions share a name. It needs +`PERPETUUM_GAMEROOT` set, and it skips rather than fails when the environment is absent — so +**a skipped run is not a pass.** Check that it actually ran. + +Reporting a violation is the whole of what it does; it changes nothing and writes nothing. + +**What it cannot check, and what therefore stays yours:** whether a robot is balanced, whether a +recipe's cost is sensible, whether tiering is coherent, whether an item is worth having, and +everything under Modules / Ammoable Equipment below — `moduleFlag`, `ammoType` and `attributeflags` +are verified against sibling items by reading, because "matches a verified sibling" is a judgement +about which sibling is the right one. A green run means the content hangs together. It does not mean +the content is good. + Before considering content complete, Claude should validate: ## Definitions diff --git a/src/Perpetuum.Tests.Integration/Content/ContentInvariantTests.cs b/src/Perpetuum.Tests.Integration/Content/ContentInvariantTests.cs new file mode 100644 index 0000000..df6c4f8 --- /dev/null +++ b/src/Perpetuum.Tests.Integration/Content/ContentInvariantTests.cs @@ -0,0 +1,165 @@ +using Microsoft.Data.SqlClient; +using Perpetuum.Tests.Integration.Infrastructure; +using Xunit; + +namespace Perpetuum.Tests.Integration.Content +{ + /// + /// Section 26 of docs/content/claude_game_content_guide.md is a validation checklist: unique + /// names, referenced fields exist, all recipe components exist, no circular dependency, tech + /// tree parents exist, coordinates do not overlap, extensions resolve, robot parts exist. Every + /// item on it is a statement about the database that a query can settle, and until now every + /// one was checked by reading. + /// + /// These tests make that checklist executable. They are read-only and run against the real + /// perpetuumsa, because the thing being checked is the content itself rather than any code path + /// — a faked data layer has nothing to say about whether a recipe names a component that exists. + /// + /// What this can prove is structural: nothing dangles, nothing duplicates, nothing points at + /// itself. What it cannot prove is that content is any good. Whether a robot is balanced, + /// whether a recipe costs a sensible amount, whether an item is worth having — none of that is + /// visible from here, and a green run must not be read as saying otherwise. + /// + [Collection(DatabaseCollection.Name)] + public class ContentInvariantTests + { + private sealed record Invariant(string Name, string Sql); + + /// + /// Each query counts violations, so zero is the passing answer for all of them. + /// + /// techtree.parentdefinition is the one that needs a qualification, and it was measured + /// rather than assumed: 21 rows carry parentdefinition = 0, which is the root-node marker + /// and not a broken reference. entitydefaults.definition is IDENTITY(1,1) and its lowest + /// live value is 1, so 0 cannot ever name a real definition. Writing that exclusion in + /// without checking would have hidden a genuine dangling parent; leaving it out reports 21 + /// healthy roots as damage. + /// + private static readonly Invariant[] Invariants = + [ + new("every production recipe belongs to a definition that exists", + """ + SELECT COUNT(*) FROM dbo.components c + WHERE NOT EXISTS (SELECT 1 FROM dbo.entitydefaults e WHERE e.definition = c.definition) + """), + + new("every production recipe component is a definition that exists", + """ + SELECT COUNT(*) FROM dbo.components c + WHERE NOT EXISTS (SELECT 1 FROM dbo.entitydefaults e WHERE e.definition = c.componentdefinition) + """), + + new("no definition is a component of itself", + """ + SELECT COUNT(*) FROM dbo.components c WHERE c.definition = c.componentdefinition + """), + + new("every tech tree parent that is not a root exists", + """ + SELECT COUNT(*) FROM dbo.techtree t + WHERE t.parentdefinition <> 0 + AND NOT EXISTS (SELECT 1 FROM dbo.entitydefaults e WHERE e.definition = t.parentdefinition) + """), + + new("every tech tree child exists", + """ + SELECT COUNT(*) FROM dbo.techtree t + WHERE NOT EXISTS (SELECT 1 FROM dbo.entitydefaults e WHERE e.definition = t.childdefinition) + """), + + new("no two tech tree nodes share a coordinate within a group", + """ + SELECT COUNT(*) FROM ( + SELECT groupID, x, y FROM dbo.techtree GROUP BY groupID, x, y HAVING COUNT(*) > 1 + ) duplicated + """), + + new("every tech tree enabler extension exists", + """ + SELECT COUNT(*) FROM dbo.techtree t + WHERE t.enablerextensionid IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM dbo.extensions x WHERE x.extensionid = t.enablerextensionid) + """), + + new("every robot template relation names a definition that exists", + """ + SELECT COUNT(*) FROM dbo.robottemplaterelation r + WHERE NOT EXISTS (SELECT 1 FROM dbo.entitydefaults e WHERE e.definition = r.definition) + """), + + new("every robot template relation names a template that exists", + """ + SELECT COUNT(*) FROM dbo.robottemplaterelation r + WHERE NOT EXISTS (SELECT 1 FROM dbo.robottemplates t WHERE t.id = r.templateid) + """), + + new("no two definitions share a name", + """ + SELECT COUNT(*) FROM ( + SELECT definitionname FROM dbo.entitydefaults GROUP BY definitionname HAVING COUNT(*) > 1 + ) duplicated + """), + ]; + + private static int Count(SqlConnection connection, string sql) + { + using SqlCommand command = connection.CreateCommand(); + command.CommandText = sql; + + return Convert.ToInt32(command.ExecuteScalar()); + } + + [RequiresGameRootFact] + public void Every_documented_content_invariant_holds() + { + DatabaseFixture fixture = new(); + using SqlConnection connection = fixture.OpenConnection(); + + List broken = []; + + foreach (Invariant invariant in Invariants) + { + int violations = Count(connection, invariant.Sql); + if (violations > 0) + { + broken.Add($"{invariant.Name}: {violations} row(s)"); + } + } + + Assert.True( + broken.Count == 0, + "Content invariants from section 26 of the content guide are violated in this database. " + + "Each count is rows, not definitions: " + + string.Join("; ", broken)); + } + + /// + /// The invariants above pass by returning zero, so a query that can never match would pass + /// for the wrong reason and stay green through any amount of broken content. The obvious way + /// for that to happen is a NULL in the referencing column: NULL = anything is never true, so + /// a NOT EXISTS written without thinking about it reports a violation that is not there, and + /// the fix people reach for — filtering NULLs out — can just as easily be written to filter + /// everything out. + /// + /// This runs the same shape against a set built in the query, holding one dangling + /// reference and one NULL, and asserts it finds exactly the dangling one. + /// + [RequiresGameRootFact] + public void The_shape_these_invariants_use_really_does_detect_a_dangling_reference() + { + DatabaseFixture fixture = new(); + using SqlConnection connection = fixture.OpenConnection(); + + int violations = Count( + connection, + """ + SELECT COUNT(*) + FROM (VALUES (1), (2), (99), (NULL)) AS child(parent) + WHERE child.parent IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM (VALUES (1), (2)) AS parent(id) WHERE parent.id = child.parent) + """); + + Assert.Equal(1, violations); + } + } +}