worldgen: -format text|json, dice-consumption invariant across formats - #372
Merged
Conversation
Adapted from ../traveller-worldgen's cmd/worldgen (#360), scoped down to what this command actually generates. t5 emits no machine-readable format anywhere, so anything downstream of the record stream must re-parse fixed-width text. - -format json emits one record per line: raw UWP characteristics (starport, size, atmosphere, hydrographics, population, government, law, techLevel) plus the rendered "uwp" convenience string and trade codes. - The shape is CLI-local (jsonWorld in cmd/worldgen), not worldgen.World.MarshalJSON: this command only ever builds a bare uwp.Profile (worldgen.Generate), never a full World — GenerateWorld needs system context (gas giants, belts, capital status) this command doesn't have and would have to fabricate to call it. A World-level JSON shape waits for a consumer that actually builds one (systemgen, survey), per the issue's own "extend as consumers appear." pbg isn't renderable at World granularity either way (belts/giants are system-level, per SecondSurvey's own doc comment). - -format never changes what is generated or how many dice a world draws — only how the same records render, so the same -seed names the same worlds under either format. TestFormatIsDiceInvariant is the one test that would catch a regression to a roll-hungrier JSON path; verified adversarially (a simulated extra roll on the json-only path made it fail before being reverted). - Golden fixture for -format json output, via #361's clitest harness. - README's flag list documents -format in prose only (no new fenced sample): ReadmeBlock's marker matching is substring-based and takes the first hit, so a JSON sample line risks colliding with the existing text sample's marker. - Updated CLAUDE.md's now-stale claim that "no test pins CLI output" — #361 already closed that. Co-Authored-By: Claude
There was a problem hiding this comment.
Pull request overview
Adds a machine-readable JSON output mode to the cmd/worldgen CLI while explicitly preserving the repo’s reproducibility contract: output format must not affect generation or dice consumption. This fits the broader CLI ecosystem that treats stdout as a stream of records and stderr as diagnostics/seed reporting.
Changes:
- Add
-format text|jsontocmd/worldgen, emitting one JSON record per generated world (one per line) whenjsonis selected. - Add golden fixture coverage for JSON output and a regression test ensuring
-formatdoes not change generated UWPs (dice-consumption invariance). - Update README and CLAUDE.md documentation to reflect the new format option and existing golden/README-parity testing guarantees.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents worldgen -format text|json and the dice-invariance guarantee. |
| cmd/worldgen/main.go | Implements -format flag and JSON streaming output for generated worlds. |
| cmd/worldgen/main_test.go | Extends CLI rejection tests to include invalid -format values. |
| cmd/worldgen/golden_test.go | Adds JSON golden fixture and format/dice-invariance test. |
| cmd/worldgen/testdata/n3seed42-json.txt | Golden fixture for -format json output. |
| CLAUDE.md | Updates repo documentation to reflect current test coverage and new worldgen format behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Correct — verified. uwp.Profile.String() already guards an invalid/zero Starport byte down to '?' (formatStarport's own doc comment: "the zero value, whose unset Starport would otherwise emit a NUL into a piped record stream"), but the JSON path read p.Starport directly, bypassing that guard. Fixed by deriving Starport from the already-guarded p.String() output instead of the raw byte. No fixture change: a valid Starport renders identically either way. Co-Authored-By: Claude
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
-format jsononcmd/worldgenemits one record per line: raw UWP characteristics plus the rendereduwpconvenience string and trade codes.../traveller-worldgen) suggestedworldgen.World.MarshalJSON, butcmd/worldgenonly ever builds a bareuwp.Profile(worldgen.Generate), never a fullWorld—GenerateWorldneeds system context (gas giants, belts, capital status) this command doesn't have and would have to fabricate. So the JSON shape here is CLI-local (jsonWorld); aWorld-level shape waits for a consumer that actually builds one (systemgen, survey), matching the issue's own "extend as consumers appear."pbgisn't renderable atWorldgranularity either way (belts/giants are system-level, perSecondSurvey's own doc comment), andru(from the issue's key list) maps to nothing in this repo's model.-formatmust not change what gets generated or how many dice get drawn —-seed 42 -format jsonand-seed 42 -format textmust describe the same worlds.TestFormatIsDiceInvariantchecks the JSON records' UWPs equal the text lines' UWPs, in order; verified adversarially (simulated an extra roll on the JSON-only path, confirmed the test fails, reverted).-formatin prose only, not a new fenced sample —ReadmeBlock's marker matching is substring-based and takes the first hit, so adding a JSON sample risked colliding with the existing text sample's marker.Closes #360.
Test plan
task check(lint + full test suite) green-format textand-format jsonoutput for the same seedTestFormatIsDiceInvariantfails on a real dice-consumption regression