From 7328400d4c8e455f57a2a56cd4b378c032418669 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 7 Sep 2026 18:27:53 +0200 Subject: [PATCH] fix: the window says when a run's record will be too big to read back The command line has warned about this since the ceiling was measured on 2026-08-26. The window said nothing at all: TooLargeToReadBack had two callers and not one of them was in internal/gui. So somebody who generated 25 000 files from the window ended up with a directory that tfg verify and tfg cleanup both refuse, and no warning anywhere. The manifest is the only authority over what may be removed, so those files could never be cleaned up by this tool again. It is the kind of parity gap D1 loses most easily. Not something the engine can do from one surface and not the other, which is what the parity guard looks for, but something one surface SAYS and the other does not - and both surfaces reach the same engine, so nothing there could see it. The observation recorded this as a question about the manifest schema, on the grounds that notes are per file and this one is per run. That premise was false, and finding out was most of the work. The command line does not read this off the manifest either: it works it out from the plan and prints it before the first byte. manifest.TooLargeToReadBack was put where it is exactly so the two surfaces could not come to different conclusions about one run. What was missing was a caller. The schema does not move and manifest_version stays 1.0. One method arrives, (*Manifest).ReadBackReach, because the count of entries carrying a note is already kept while entries are added. Working it out a second time would mean walking every file again and getting it subtly wrong when a failed entry gains a note of its own. The window says it in two places, because it cannot say anything in the middle of a run - a widget touched from a worker is a race, and two of those were found on CI: - after Preview, which is the window's answer to --dry-run. engine.Run with DryRun builds the whole document, so the answer is there for the asking. - when a run finishes, immediately after the line saying what the run did and ahead of any other note. This is the one that matters: Preview is a button somebody may never press, and that person is the one left with the directory. Four guards, four mutations. The pair for Preview was written wrong first and the probe said so. It set a 200 B size while the window was on its default format, which is the first in the registry - avif - so both previews were REFUSED. The negative half passed while proving nothing, because a screen saying "check the settings marked above" says nothing about a manifest ceiling either. Both now choose txt and both assert the preview was accepted before reading anything into what it said. The guard for a finished run costs 26 s, because it really writes 22 345 files. runFinished has no cheaper seam, and this is the only guard covering the case the observation is actually about. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 16 ++ internal/guard/windowmanifestreach_test.go | 222 +++++++++++++++++++++ internal/gui/text/locale/en.json | 4 + internal/gui/text/text.go | 17 ++ internal/gui/window/run.go | 24 ++- internal/gui/window/runreport.go | 32 +++ internal/manifest/manifest.go | 20 ++ 7 files changed, 329 insertions(+), 6 deletions(-) create mode 100644 internal/guard/windowmanifestreach_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e2de7fd..7a4e4e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -299,6 +299,22 @@ because it turns other people's test suites red. ### Fixed +- **The window now warns when a run's record will be too big for this build to + read back.** The command line has said this since the ceiling was measured. + The window said nothing at all, so somebody who generated 25 000 files from it + was left with a directory that `tfg verify` and `tfg cleanup` both refuse - + and the manifest is the only authority over what may be removed, so those + files could never be cleaned up by this tool again. + + It appears in two places, because the window cannot speak in the middle of a + run: under Preview, which is the window's answer to `--dry-run`, and again + when a run finishes. The second is the one that matters, since Preview is a + button somebody may never press. It comes straight after the line saying what + the run did, ahead of any other note. + + The run itself still works and is still not refused. What was missing was that + nobody was told. + - **`tfg verify` no longer calls another run's files "extra".** A directory is allowed to hold more than one run - that is what `output.manifest` is for - and verifying one of them reported every file the other had written as a file diff --git a/internal/guard/windowmanifestreach_test.go b/internal/guard/windowmanifestreach_test.go new file mode 100644 index 0000000..cbecd4b --- /dev/null +++ b/internal/guard/windowmanifestreach_test.go @@ -0,0 +1,222 @@ +package guard + +import ( + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// The window says the same thing the command line says about a record too big +// to read back. +// +// Observation O184, measured on 2026-09-06: TooLargeToReadBack had two callers, +// both in internal/cli and internal/manifest, and NOT ONE in internal/gui. A +// person who generated 25 000 files from the window was told nothing at all, +// and was left with a directory that tfg verify and tfg cleanup both refuse - +// the manifest is the only authority over what may be removed, so a manifest +// that cannot be read is a set of files with no owner. +// +// It is the kind of parity gap D1 loses most easily. Not something the engine +// can do from one surface and not the other, which is what the parity guard +// looks for, but something one surface SAYS and the other does not. +// +// The note was written down as a question about the manifest schema, on the +// grounds that notes are per file and this one is per run. It is not. The +// command line does not read this off the manifest either - it works it out +// from the plan and prints it before the first byte - and manifest.TooLarge- +// ToReadBack was put where it is exactly so the two surfaces could not come to +// different conclusions about one run. What was missing was a caller. + +// overTheCeiling is a file count whose manifest this build would refuse. +// +// Worked out from the estimate rather than written here, for the reason the +// command line guard beside it gives: a guard carrying its own copy of a limit +// goes stale the day somebody changes the real one, and says nothing while it +// does. +func overTheCeiling(t *testing.T) int { + t.Helper() + over := int(manifest.MaxBytes/manifest.BytesPerEntry) + 1000 + if _, tooBig := manifest.TooLargeToReadBack(over, 0); !tooBig { + t.Fatalf("%d entries was not judged too large, so this guard would prove nothing", over) + } + return over +} + +// previewOf presses Preview for a run of count files and gives back what the +// screen said. +// +// It REFUSES to return a refusal, and that is the whole reason it exists. The +// first version of the pair below set a size the default format will not take - +// the window opens on the first format in the registry, which is avif, and +// 200 B is far under what a picture needs. Both previews were turned down, so +// the negative half passed while proving nothing: a screen that says "check the +// settings marked above" says nothing about a manifest ceiling either. +func previewOf(t *testing.T, count string) string { + t.Helper() + content, w, host := screenInAWindowWithHost(t, text.TabOneTarget()) + + // txt rather than whatever the window opens on, for two reasons. The size + // below has to be one the format takes, and planning twenty two thousand + // pictures would encode twenty two thousand pictures - png, jpg, gif and + // avif all do that while planning. + picker, ok := controlUnder(content, text.FieldFormat()).(*parts.Chooser) + if !ok { + t.Fatal("the format field is not a list to choose from, so this guard read the wrong tree") + } + picker.SetSelected("txt") + + // Small files, because what is being asked about is the number of ENTRIES + // rather than the number of bytes. A preview writes nothing either way. + fill(t, content, text.FieldSize(), "200b") + fill(t, content, text.FieldCount(), count) + + press(t, content, text.ButtonPreview()) + // This preview is accepted, so it answers from a worker. Joined before the + // status line is read - see join. + join(host) + settle(content, w) + + _, status := runMessages(content) + if status == nil { + t.Fatal("the screen has no status line, so this guard read the wrong tree") + } + // Matched on the tail of the preview's own sentence, the way the action bar + // guard does it, so this cannot be satisfied by a refusal. + marker := text.PreviewCost(1, nil, "1 B") + tail := marker[strings.LastIndex(marker, " ")+1:] + if !strings.Contains(status.Text, tail) { + t.Fatalf("the preview of %s files was not accepted, so nothing here was asked about the manifest.\nIt said:\n%s", + count, status.Text) + } + return status.Text +} + +// runOf presses Generate rather than Preview, and gives back what the screen +// said when it finished. +// +// It exists because the preview is OPTIONAL. Somebody who presses Generate +// straight away never sees the preview's answer, and that person is exactly the +// one observation O184 is about - they end up with a directory nothing in this +// toolset can read or clean. The window cannot say anything in the middle of a +// run, so the end of the run is the only place left. +// +// It writes files, which is why this is the one guard here that does. Twenty +// two thousand of them at 200 B, into a directory that goes away with the test. +func runOf(t *testing.T, count string) string { + t.Helper() + content, w, host := screenInAWindowWithHost(t, text.TabOneTarget()) + + picker, ok := controlUnder(content, text.FieldFormat()).(*parts.Chooser) + if !ok { + t.Fatal("the format field is not a list to choose from, so this guard read the wrong tree") + } + picker.SetSelected("txt") + fill(t, content, text.FieldSize(), "200b") + fill(t, content, text.FieldCount(), count) + fill(t, content, text.FieldOutputDir(), t.TempDir()) + + press(t, content, text.ButtonGenerate()) + join(host) + settle(content, w) + + _, status := runMessages(content) + if status == nil { + t.Fatal("the screen has no status line, so this guard read the wrong tree") + } + // The run has to have HAPPENED. A refused run says nothing about a + // manifest either, and a guard that cannot tell those apart is the shape + // this project has recorded as passing without reaching the code. + if !strings.Contains(status.Text, text.Written(0)[strings.LastIndex(text.Written(0), " ")+1:]) { + t.Fatalf("the run of %s files did not finish, so nothing here was asked about the manifest.\nIt said:\n%s", + count, status.Text) + } + return status.Text +} + +// warningAbout is the fixed half of the sentence, without the two numbers. +// +// Taken from the text package rather than typed here, so a reworded warning +// does not quietly stop being checked. +func warningAbout(t *testing.T) string { + t.Helper() + marker := text.ManifestTooLargeToRead("SIZE", "LIMIT") + at := strings.Index(marker, "SIZE") + if at < 0 { + t.Fatal("the warning does not carry the size it was given, so this guard cannot find its fixed half") + } + return marker[:at] +} + +func TestTheWindowSaysWhenItsManifestWillBeTooBigToReadBack(t *testing.T) { + said := previewOf(t, itoa(overTheCeiling(t))) + if !strings.Contains(said, warningAbout(t)) { + t.Errorf("a preview of %d files said nothing about the record being too big to read back.\n"+ + "The command line has said this since 2026-08-26. Somebody who does the same from the window "+ + "gets a directory that neither Verify nor Clean up can read, and no warning.\nIt said:\n%s", + overTheCeiling(t), said) + } +} + +// And a run that was never previewed says it too, which is the case that +// matters most. +// +// The preview is a button somebody may not press. The warning has to reach the +// person who pressed Generate and nothing else, because they are the one left +// with the directory. +func TestAFinishedRunInTheWindowSaysItsManifestIsTooBigToReadBack(t *testing.T) { + said := runOf(t, itoa(overTheCeiling(t))) + if !strings.Contains(said, warningAbout(t)) { + t.Errorf("a finished run of %d files said nothing about the record being too big to read back.\n"+ + "That directory now has a manifest neither Verify nor Clean up can read, and nobody was told.\nIt said:\n%s", + overTheCeiling(t), said) + } + // The line somebody pressed the button for stays first. The room for these + // messages is a ceiling and the message scrolls inside it. + if first := strings.SplitN(said, "\n", 2)[0]; strings.Contains(first, warningAbout(t)) { + t.Errorf("the warning took the first line from the outcome:\n%s", said) + } +} + +// A run this build CAN read back stays quiet. +// +// Without this the guard above passes on a window that warns about every run, +// which teaches somebody to stop reading the line - the same reason the command +// line has this pair rather than only the first half. +func TestAnOrdinaryPreviewSaysNothingAboutTheManifestCeiling(t *testing.T) { + said := previewOf(t, "100") + if strings.Contains(said, warningAbout(t)) { + t.Errorf("a hundred files drew the warning about the record being too big:\n%s", said) + } +} + +// Both surfaces judge the same run the same way. +// +// The window reads the answer off the document a dry run builds, and the +// command line works it out from the plan before anything is written. Two paths +// to one number, and what makes two paths acceptable is that they go through +// one predicate. Asked at the boundary, which is the only place a disagreement +// would show. +func TestBothSurfacesJudgeTheSameRunTheSameWay(t *testing.T) { + for _, entries := range []int{ + int(manifest.MaxBytes / manifest.BytesPerEntry), + int(manifest.MaxBytes/manifest.BytesPerEntry) + 1, + } { + _, fromThePlan := manifest.TooLargeToReadBack(entries, 0) + + m := manifest.New("testing-files-generator", "0.0.0-dev", "run_x", "tfg generate", 1, "linux", "amd64") + for i := 0; i < entries; i++ { + m.Add(manifest.File{Path: "f.txt", Materialized: true}) + } + _, fromTheDocument := m.ReadBackReach() + + if fromThePlan != fromTheDocument { + t.Errorf("at %d entries the plan says %v and the document says %v.\n"+ + "The command line answers from the first and the window from the second, so one run "+ + "would be warned about on one surface and not on the other", + entries, fromThePlan, fromTheDocument) + } + } +} diff --git a/internal/gui/text/locale/en.json b/internal/gui/text/locale/en.json index 3b8d654..c154a75 100644 --- a/internal/gui/text/locale/en.json +++ b/internal/gui/text/locale/en.json @@ -248,6 +248,10 @@ "description": "Shown in the window. Carries one value, {{.Path}}, which has to stay spelled exactly that way.", "other": "the files were written and the manifest could not be saved to {{.Path}}" }, + "ManifestTooLargeToRead": { + "description": "Shown in the window. Carries these values, each of which has to stay spelled exactly that way: {{.Size}}, {{.Limit}}.", + "other": "this run's record is about {{.Size}} and this build reads at most {{.Limit}}, so Verify and Clean up will not be able to read it. Split the run to keep each record readable." + }, "NotAWholeNumber": { "description": "Shown in the window. Carries these values, each of which has to stay spelled exactly that way: {{.Field}}, {{.Value}}.", "other": "{{.Field}} is {{.Value}}, which is not a whole number. Write the digits out, such as 1 or 500" diff --git a/internal/gui/text/text.go b/internal/gui/text/text.go index 597f319..88db185 100644 --- a/internal/gui/text/text.go +++ b/internal/gui/text/text.go @@ -96,6 +96,23 @@ func PreviewCost(count int, formats []string, total string) string { say("PreviewNothingWritten", "nothing written yet") } +// ManifestTooLargeToRead is said when a run will write a record this build +// cannot read back. +// +// The command line has printed this since 2026-08-26 and the window said +// nothing at all, which is the parity gap observation O184 names. A person who +// generates 25 000 files from a window gets a directory that tfg verify and +// tfg cleanup both refuse - and the manifest is the only authority over what +// may be deleted, so nothing in this toolset can ever remove those files. +// +// A note rather than a refusal, which is the owner's decision from that day and +// is unchanged here. The run works. What was missing was that nobody was told. +func ManifestTooLargeToRead(size, limit string) string { + return sayf("ManifestTooLargeToRead", + "this run's record is about {{.Size}} and this build reads at most {{.Limit}}, so Verify and Clean up will not be able to read it. Split the run to keep each record readable.", + map[string]any{"Size": size, "Limit": limit}) +} + // PreviewFreeSpace follows PreviewCost when the disk could be measured. It is // a separate fact because a disk we cannot read has to say nothing at all // rather than invent a number. diff --git a/internal/gui/window/run.go b/internal/gui/window/run.go index aeade27..f66f6c6 100644 --- a/internal/gui/window/run.go +++ b/internal/gui/window/run.go @@ -664,25 +664,30 @@ func (r *runner) onPreview() { // Do rather than DoAndWait, for the same reason startRun gives: the // interface thread must never be left waiting on a worker. r.holdBeforeFinishing() - fyne.Do(func() { r.previewFinished(nil, opt, planErr) }) + fyne.Do(func() { r.previewFinished(nil, nil, opt, planErr) }) close(done) return } - _, runErr := engine.Run(ctx, planned, opt) + res, runErr := engine.Run(ctx, planned, opt) r.holdBeforeFinishing() - fyne.Do(func() { r.previewFinished(planned, opt, runErr) }) + fyne.Do(func() { r.previewFinished(res, planned, opt, runErr) }) close(done) }() } // previewFinished is the end of a preview, back on the interface thread. -func (r *runner) previewFinished(planned []engine.PlannedFile, opt engine.Options, runErr error) { +// +// The result is carried across as well as the plan, and that is what lets a +// preview warn about a record too big to read back. A dry run builds the whole +// document - see manifestReachNote - so the answer is there for the asking +// rather than something the window would have to work out for itself. +func (r *runner) previewFinished(res *engine.Result, planned []engine.PlannedFile, opt engine.Options, runErr error) { r.setBusy(false, false) if runErr != nil { r.refuse(runErr) return } - r.say(previewText(planned, opt.OutDir)) + r.say(append([]string{previewText(planned, opt.OutDir)}, manifestReachNote(res)...)...) } // formatsOf is what kinds of file the run would produce, each named once. @@ -837,7 +842,14 @@ func (r *runner) runFinished(res *engine.Result, runErr, saveErr error) { // and not only in the manifest - "the manifest says which ones" is an // answer in a terminal and an instruction to open a file with ten thousand // entries in a window. - r.say(append([]string{outcomeText(res, runErr)}, notesOf(res)...)...) + // + // The warning about a record too big to read back comes SECOND, ahead of + // the per file notes, and that order is the same lesson the command line + // learned on 2026-09-06: it is the one line standing between somebody and a + // directory nothing in this toolset can ever clean up, and it was being + // buried under notes about a label that did not fit. + said := append([]string{outcomeText(res, runErr)}, manifestReachNote(res)...) + r.say(append(said, notesOf(res)...)...) r.toneOfOutcome(res, runErr) r.offerTheFolder(res) } diff --git a/internal/gui/window/runreport.go b/internal/gui/window/runreport.go index 5b1a947..baecbd4 100644 --- a/internal/gui/window/runreport.go +++ b/internal/gui/window/runreport.go @@ -7,6 +7,7 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/engine" "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" ) // What a run tells the person while it goes and when it ends. @@ -31,6 +32,37 @@ func previewText(planned []engine.PlannedFile, outDir string) string { return line } +// manifestReachNote is the window's half of the warning the command line prints +// before the first byte. +// +// Observation O184: the command line has said this since 2026-08-26 and the +// window said nothing at all, so a person generating 25 000 files from a window +// was left with a directory that neither Verify nor Clean up can read - and the +// manifest is the only authority over what may be removed. A parity gap in +// quality rather than in what the engine can do, which is the kind D1 is +// easiest to lose. +// +// Read off the document rather than worked out here, and off the SAME predicate +// the command line uses, which is what manifest.TooLargeToReadBack exists for. +// The two surfaces cannot come to different conclusions about one run. +// +// A preview reaches this too. engine.Run with DryRun adds an entry for every +// planned file, so the document a preview produces is the document the run +// would produce, minus the bytes on the disk. That is why one shape serves +// both, and why the window can answer before anything is written even though it +// cannot say a word in the middle of a run. +func manifestReachNote(res *engine.Result) []string { + if res == nil || res.Manifest == nil { + return nil + } + size, over := res.Manifest.ReadBackReach() + if !over { + return nil + } + return []string{text.ManifestTooLargeToRead( + core.HumanBytes(size), core.HumanBytes(manifest.MaxBytes))} +} + // progressText is the line under the bar. Bytes rather than files, because one // large file is a run where the file count says nothing for minutes. func progressText(p engine.Progress, elapsed time.Duration) string { diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index a2b5744..370ff70 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -527,6 +527,26 @@ func TooLargeToReadBack(entries, withNotes int) (int64, bool) { return n, n > MaxBytes } +// ReadBackReach is what this document will weigh and whether this build would +// refuse to read it. +// +// The same question TooLargeToReadBack answers, asked of a manifest that +// exists rather than of a plan that has not run. It is here because the two +// callers had nowhere else to get the count of entries carrying a note from - +// that is kept while entries are added, so working it out again would mean +// walking every file a second time and getting it subtly wrong when a failed +// entry gains a note of its own. +// +// The window needs this and the command line does not. The command line says +// its piece BEFORE the first byte, from the plan, where no manifest exists yet. +// A window cannot say anything in the middle of a run - a widget touched from a +// worker is a race, and two of those were found on CI - so it says it when the +// run ends, off the document the run actually produced. Both go through +// TooLargeToReadBack, which is where that answer is settled for both surfaces. +func (m *Manifest) ReadBackReach() (int64, bool) { + return TooLargeToReadBack(len(m.Files), m.notedFiles) +} + // TooLargeError is returned for a manifest past MaxBytes. type TooLargeError struct { Path string