diff --git a/CHANGELOG.md b/CHANGELOG.md index 88474a9..79069de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -299,6 +299,15 @@ because it turns other people's test suites red. ### Fixed +- **A refusal about a size now names the setting you actually wrote.** A target + using `size-range` was refused at `targets[1].size`, a key that recipe does + not have, so `validate --json` sent a script - and the window sent a person - + to a box that was not there. It now reads `targets[1].size-range`. Targets + using `size` are unchanged. + + The wording of every refusal is byte for byte what it was. Only the address + moved. + - **`verify` no longer calls a half-written manifest a file it knows nothing about.** A run killed outright can leave `.tfg-writing` behind. `verify` reported it as `extra`, the word it uses for a file somebody else put diff --git a/internal/core/setting.go b/internal/core/setting.go index de310e4..e9b64d1 100644 --- a/internal/core/setting.go +++ b/internal/core/setting.go @@ -84,6 +84,20 @@ func articleFor(name string) string { // than either surface's own word for the same thing. const SettingSize = "size" +// SettingSizeRange is the recipe key a refusal about a RANGE of sizes is about. +// +// A target that draws its sizes from a range has no "size" key at all, so an +// address naming one sends somebody to a box that is not on their screen and +// not in their file. Measured on 2026-09-06: a recipe carrying "size-range: +// 143-200" was refused with "at": "targets[1].size", naming a setting the +// person had not written. +// +// The refusal itself comes from the format, which knows only that a number of +// bytes is out of reach and cannot know which key carried that number. So the +// substitution happens where the target is in hand - see atTarget in the +// engine. +const SettingSizeRange = "size-range" + // SettingErrorf is a refusal that names the setting it is about through a slot, // so each surface reads it in its own words. // diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 2a02fb7..c0cf03c 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -78,8 +78,31 @@ type Target struct { // container was told to hold - would otherwise fail on some runs and not // others, depending on what came out of the seed. A tool whose whole promise // is that the same seed gives the same run cannot have an error that appears -// and disappears. So the low end is planned first and the range either works -// for every file or for none. +// and disappears. +// +// THE LOW END IS NOT THE WHOLE ANSWER, and this comment claimed it was +// until 2026-09-06. It said the range "either works for every file or for +// none", and the code does not provide that. The check here is sufficient only +// if a format's reachable sizes are one unbroken interval starting at its +// minimum, and for four of them they are not: PNG has an unreachable band of +// eleven byte counts immediately above every picture's encoded size, because +// the smallest padding chunk costs twelve bytes, and the OPC three declare the +// same shape between the comment capacity and the smallest extra part. +// +// So a size DRAWN into such a band is refused later, by the per file plan, and +// whether that happens depends on the count. Measured on 2026-09-06, one 64x64 +// PNG recipe at one seed with size-range 143-200: counts 1 and 2 are accepted, +// counts 3, 5, 8, 12, 20 and 40 are refused. The low end moves with the seed +// too - 144, 143, 144 B at seeds 1, 2 and 3 - so judging file 0's band says +// nothing about file 2's. +// +// The bytes are stable under a raised count and that was verified, so rule 2 +// holds for CONTENT. What is not stable is whether the run happens at all. +// Closing that needs the format to declare its unreachable bands so the whole +// interval can be judged before anything is drawn, which is a change to +// format.Descriptor and the owner's call. Until then the refusal at least +// names the key the recipe carries - see atTarget - rather than pointing at a +// "size" setting a range target does not have. // // The judge is the generator itself rather than a second copy of its rules // here. A copy would be a place for the two to disagree, and the disagreement @@ -392,7 +415,7 @@ func PlanContext(ctx context.Context, targets []Target, opt Options) ([]PlannedF // deliberately leaves alone. desc, err := settleTarget(t, opt, seen) if err != nil { - return nil, atTarget(i+1, err) + return nil, atTarget(i+1, t, err) } targetSeed := core.TargetSeed(opt.Seed, t.ID) diff --git a/internal/engine/errors.go b/internal/engine/errors.go index 6ec192b..08917b4 100644 --- a/internal/engine/errors.go +++ b/internal/engine/errors.go @@ -152,7 +152,7 @@ const ( // Inventing a position for it would put a message about the whole run under // one batch of twenty, which is worse than leaving it at the foot of the form // where a message about the run belongs. -func atTarget(position int, err error) error { +func atTarget(position int, t *Target, err error) error { var about interface{ AboutSetting() string } if !errors.As(err, &about) || about.AboutSetting() == "" { return err @@ -163,6 +163,15 @@ func atTarget(position int, err error) error { if core.AddressNamesATarget(setting) { return err } + // A refusal about a size belongs to the key that carried the number. The + // format cannot know which one that was - it is handed a count of bytes and + // answers about bytes - so the substitution happens here, where the target + // is in hand. Without it a recipe written with "size-range" is refused at + // "targets[1].size", which is a box that is not on the screen and not in + // the file. Measured on 2026-09-06. + if setting == core.SettingSize && t != nil && t.SizeIsRange { + setting = core.SettingSizeRange + } return &addressedError{err: err, at: core.TargetAddress(position, setting)} } diff --git a/internal/engine/plantarget.go b/internal/engine/plantarget.go index 3b3bd73..f47162b 100644 --- a/internal/engine/plantarget.go +++ b/internal/engine/plantarget.go @@ -44,12 +44,12 @@ func (pl *planning) files(ctx context.Context, t *Target, desc format.Descriptor Properties: t.Properties, }) if err != nil { - return atTarget(position, err) + return atTarget(position, t, err) } name, err := renderName(t, desc, idx) if err != nil { - return atTarget(position, err) + return atTarget(position, t, err) } // Two files heading for one name means one of them would be // destroyed by the other, and the manifest would still describe diff --git a/internal/guard/sizerange_test.go b/internal/guard/sizerange_test.go index fc9a203..b50da46 100644 --- a/internal/guard/sizerange_test.go +++ b/internal/guard/sizerange_test.go @@ -1,8 +1,11 @@ package guard import ( + "bytes" + "context" "crypto/sha256" "encoding/hex" + "encoding/json" "os" "path/filepath" "strings" @@ -321,3 +324,91 @@ targets: t.Errorf("all six archives came out the same size, so nothing is being drawn") } } + +// A refusal about a size names the key the recipe actually carries. +// +// The format is handed a count of bytes and answers about bytes, so every size +// refusal it produces says "size" - the only size key it knows. A target that +// draws from a range has no "size" key at all. Measured on 2026-09-06: a recipe +// carrying "size-range: 143-200" was refused at "targets[1].size", sending +// somebody to a box that was neither on their screen nor in their file. +// +// NOT COVERED BY TestEveryRecipeRefusalSaysWhichSettingItIsAbout, and that was +// checked rather than assumed. Its table has a "size-range" case already, but +// that refusal comes from the recipe READER, which knows which key it was +// reading. This one comes from underneath, and the table's runner never reaches +// it - the two cases were tried there first and the recipe came back valid, +// because nothing in that path plans a file. +// +// The pair matters more than either half. A build that answered "size-range" +// for every size refusal would pass the first case and misaddress every +// ordinary target in the tree. +func TestASizeRefusalIsAddressedToTheKeyTheRecipeCarries(t *testing.T) { + cases := []struct { + name string + src string + want string + }{ + { + name: "a range the format cannot deliver", + src: `version: 1 +targets: + - id: a + format: pdf + count: 40 + size-range: 10-8kb +`, + want: "targets[1].size-range", + }, + { + name: "a plain size the format cannot deliver", + src: `version: 1 +targets: + - id: a + format: pdf + count: 1 + size: 10 +`, + want: "targets[1].size", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + path := writeRecipe(t, dir, c.src) + + var out, errOut bytes.Buffer + if code := cli.Run(context.Background(), + []string{"validate", path, "--json"}, &out, &errOut); code == cli.ExitOK { + t.Fatalf("this recipe was meant to be refused and validate was happy with it:\n%s", + out.String()) + } + + var report struct { + Problems []struct { + What string `json:"what"` + At string `json:"at"` + } `json:"problems"` + } + if err := json.Unmarshal(errOut.Bytes(), &report); err != nil { + t.Fatalf("the report is not readable as JSON: %v\n%s", err, errOut.String()) + } + // Asserted rather than assumed. A report with no problems in it + // would pass the loop below by never entering it. + if len(report.Problems) == 0 { + t.Fatalf("the report carries no problems at all:\n%s", errOut.String()) + } + + for _, pr := range report.Problems { + if pr.At == c.want { + continue + } + t.Errorf("the refusal %q is addressed to %q and belongs at %q.\n"+ + "What to do: a refusal about a size carries the key that held the number, "+ + "so a window can mark the box somebody can actually change.", + pr.What, pr.At, c.want) + } + }) + } +} diff --git a/internal/recipe/compose.go b/internal/recipe/compose.go index 59a3786..0dd7bfe 100644 --- a/internal/recipe/compose.go +++ b/internal/recipe/compose.go @@ -322,8 +322,8 @@ const ( KeyID = "id" KeyFormat = "format" KeyCount = "count" - KeySize = "size" - KeySizeRange = "size-range" + KeySize = core.SettingSize + KeySizeRange = core.SettingSizeRange KeyBoundary = "boundary" KeyName = "name" KeyGroup = "group"