From d286f449a94da86069dc5d36683a2636bc5c3cbc Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 3 Sep 2026 14:32:55 +0200 Subject: [PATCH 1/3] format: a CSV quotes the fields that need it, and you choose which Adds quote_style to the csv format, taking minimal, all or none. The default is minimal, which changes the bytes of every table this tool writes - the description column used to be quoted on every row and is quoted now only when it carries the separator. This is a breaking change under D11 and it is deliberate. Measured on a 4 kB table at seed 7 before the change: 9 of 44 rows carry a description with no separator in it, because the phrase is three to seven words and drops a separator every third one. Those nine lose their quotes. Sizes are unchanged and every reader that took these files still takes them. The values are the RFC 4180 vocabulary and nothing outside it. A fourth name that preserved today's bytes was rejected: the release this belongs to closes with a major bump either way, so a clean vocabulary costs nothing now and a fourth name would have been carried forever. Two things are not obvious from the list of values. none changes the CONTENT, not only the punctuation. An unquoted field cannot hold a separator without ending early, so the description stops carrying one - in the phrase and in the padding both. A value that only removed the quotes would produce a ragged row at exactly the right size. And the closing row is built to the byte, so under minimal the decision to quote changes the length that the decision depends on. Measured over 59 sizes: the padding first carries a separator at 30 B of description, so the ambiguous band is two sizes per dialect. It is resolved by MEASURING - the quoted length is built first, and if it carries the separator the quotes are earned, otherwise the description is rebuilt to the full room with the separator withheld. A threshold constant was rejected as arithmetic that has to keep agreeing with the bytes beside it. The floor moves with the setting, the way the dialect already does: 115 B rather than 117 under minimal and none, 139 B under all. Twelve combinations, eight distinct floors, all measured with the binary. all quotes the header too, because a header is a row of fields and a writer told to quote everything quotes those as well. The alternative left the structural checker with an "except the first row" exception, which is where a defect hides. The checker is now told the style and judges it, with negative controls in both directions. That matters more here than for the other axes: every one of the three styles produces a well formed file, so nothing about the table gives the style away and a rubber stamp would have been silent. Verified: 1152 files swept over every size from 115 to 260 B across three styles and two delimiters - exact size, six columns under Python's csv module, quoting matching the style, and no needless quote under minimal. All 48 dialect combinations through the structural checker. LibreOffice Calc headless reads all three variants as six columns on every row, which closes D4 for the new values. 16 of 16 mutations caught. Two of those mutations were NOT CAUGHT at first, and the fault was in the guard: both only shift the ANNOUNCED floor, which is invisible elsewhere because Shortest is the worst draw and a real row sits some forty bytes under it. The only handle is the count of distinct floors, and it was written as six where the axes make eight. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 30 ++ README.md | 3 +- internal/format/csvfile/csv.go | 132 ++++-- internal/format/csvfile/dialect.go | 124 +++++- internal/guard/csvdialect_test.go | 387 +++++++++++------- internal/guard/generatorbytes_test.go | 13 +- internal/guard/parity_test.go | 1 + internal/guard/testdata/generator-golden.json | 19 +- internal/guard/textformats_test.go | 2 +- internal/oracle/strict.py | 54 ++- web/public/formats/index.html | 7 +- web/public/pl/formaty/index.html | 7 +- 12 files changed, 596 insertions(+), 183 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9205ce8..2657860 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,36 @@ because it turns other people's test suites red. ### Breaking +- **A generated `.csv` quotes only the fields that need it, so its bytes are + different.** Sizes are unchanged. Every size that worked before still works, + every reader that took these files still takes them, and the file is still + RFC 4180. + + The description column used to be quoted on every row. It is quoted now only + when it carries the separator, which is the one thing that makes a quote + necessary. The description is three to seven words and drops a separator + every third one, so a short one carries none - measured on a 4 kB table, + **9 of its 44 rows** lost their quotes. + + This arrives as a new setting, `quote_style`, which takes `minimal`, `all` or + `none`: + + - `minimal` is the new default and is what a spreadsheet writes. + - `all` wraps every field on every row, the header included. + - `none` wraps nothing. It also stops the description carrying the separator, + because an unquoted field cannot hold one without ending early - so this + value changes what the file says and not only how it is punctuated. + + **The smallest `.csv` is 115 B rather than 117**, because the shortest row + has an empty description and an empty field needs no quotes. With + `quote_style=all` the smallest is 139 B. `tfg formats` prints the current + numbers. + + **A suite pinning `.csv` hashes will go red once and then stay green.** There + is no switch back to the old bytes: they were not any of the three styles RFC + 4180 describes, and carrying a fourth name for them forever costs more than + the one red run. + - **Six formats have different bytes, because the tool is built with Go 1.27 now.** Sizes are unchanged. Every size that worked before still works, every reader that took these files still takes them, and the same sizes are diff --git a/README.md b/README.md index 4e741fb..62d9f12 100644 --- a/README.md +++ b/README.md @@ -467,7 +467,8 @@ recipe. `tfg formats ` prints the allowed range or list for each: | `docx` | `paragraphs` | | `xlsx` | `rows`, `columns` | | `pptx` | `slides` | -| `csv`, `json`, `xml`, `html`, `md`, `log`, `txt`, `svg` | none | +| `csv` | `delimiter`, `line_ending`, `header`, `quote_style` | +| `json`, `xml`, `html`, `md`, `txt`, `svg` | none | ``` tfg generate --format jpg --size 500kb --set width=1920 --set height=1080 --set quality=85 diff --git a/internal/format/csvfile/csv.go b/internal/format/csvfile/csv.go index 2b4bc40..3d95a2a 100644 --- a/internal/format/csvfile/csv.go +++ b/internal/format/csvfile/csv.go @@ -42,9 +42,10 @@ const ( // drawn from below guarantees it. amountWidth = 9 - // closingQuote ends the description. What follows it is the row ending, - // which the dialect decides, so the two are no longer one constant. - closingQuote = `"` + // quoteMark wraps a field. It is the one character RFC 4180 gives a value + // for holding a separator inside it, and how many of them a row carries is + // what quote_style decides. + quoteMark = '"' // maxRowDigits bounds the width of the row number. A row is at least one // byte, so a file can never hold more rows than it has bytes, and a size is @@ -54,21 +55,23 @@ const ( maxRowDigits = 19 // fixedBeforeEnding is every byte of a row except the row number, the name - // (which also forms the address), the description and the row ending. A - // constant expression, so it cannot drift away from the template above. + // (which also forms the address), the description, the quotes and the row + // ending. A constant expression, so it cannot drift away from the template + // above. // // The five separators count one byte each, which is a fact about the // separators offered rather than an assumption: every one of them is a // single byte, and dialect.go says so where they are declared. fixedBeforeEnding = 5 /* separators */ + len(emailDomain) + amountWidth + - len(createdDate) + 1 /* the opening quote */ + len(closingQuote) + len(createdDate) ) -// fixedWidth is fixedBeforeEnding plus the row ending, which the dialect -// decides. A CRLF row costs one byte more than an LF one, on every row, which -// is why the minimum moves with this setting. +// fixedWidth is fixedBeforeEnding plus the quotes and the row ending, both of +// which the dialect decides. A CRLF row costs one byte more than an LF one on +// every row, and quoting every field costs two bytes per column, which is why +// the minimum moves with either setting. func fixedWidth(d dialect) int64 { - return int64(fixedBeforeEnding + len(d.eol)) + return int64(fixedBeforeEnding + d.quotes.quoteBytes() + len(d.eol)) } func init() { @@ -100,8 +103,8 @@ func init() { // name and the manifest carry it instead. Label: format.LabelExternalOnly, Oracle: "python-csv", - // Quoting, column count and column types come later. Declaring none of - // them now makes a recipe asking for one fail loudly. + // Column count and column types come later. Declaring neither of them + // now makes a recipe asking for one fail loudly. Properties: properties(), GeneratorVersion: generatorVersion, Generator: generator{}, @@ -146,6 +149,7 @@ func (generator) Plan(r format.Request) (format.Plan, error) { "line_ending": d.lineEndingID, "separator": string(d.sep), "header": d.header, + "quote_style": d.quotes.id, "columns": len(columnNames), // Stated even though it is always false here, so a test can assert // on it without knowing which formats carry a label internally. @@ -186,6 +190,15 @@ func (generator) Write(ctx context.Context, w io.Writer, p format.Plan) error { type rows struct { next int64 dia dialect + + // scratch holds the description while it is being asked whether it needs + // quotes. It cannot be written straight into the row, because the answer + // decides whether a quote goes in FRONT of it. + // + // Reused rather than allocated per row. A table of any size is millions of + // rows and the resource guard measures exactly that. It stays small: the + // closing row is the longest and is bounded by twice the shortest row. + scratch []byte } // Shortest is the smallest row this builder can close a file with: the widest @@ -229,33 +242,100 @@ func (r *rows) append(dst []byte, rng *rand.Rand, want int64) []byte { cents := rng.IntN(100) sep := r.dia.sep + q := r.dia.quotes + dst = q.mark(dst) dst = strconv.AppendInt(dst, r.next, 10) + dst = q.mark(dst) dst = append(dst, sep) + dst = q.mark(dst) dst = append(dst, name...) + dst = q.mark(dst) dst = append(dst, sep) + dst = q.mark(dst) dst = append(dst, name...) dst = append(dst, emailDomain...) + dst = q.mark(dst) dst = append(dst, sep) + dst = q.mark(dst) dst = strconv.AppendInt(dst, int64(whole), 10) dst = append(dst, '.') if cents < 10 { dst = append(dst, '0') } dst = strconv.AppendInt(dst, int64(cents), 10) + dst = q.mark(dst) dst = append(dst, sep) + dst = q.mark(dst) dst = append(dst, createdDate...) - dst = append(dst, sep, '"') + dst = q.mark(dst) + dst = append(dst, sep) + + return r.appendDescription(dst, rng, want, int64(len(dst)-start)) +} +// appendDescription writes the last field and ends the row. +// +// want below zero means a natural row, any other value is the exact length the +// whole row must have. used is what the row has spent already, measured rather +// than worked out beside the bytes. +func (r *rows) appendDescription(dst []byte, rng *rand.Rand, want, used int64) []byte { if want < 0 { - dst = appendPhrase(dst, rng, 3+rng.IntN(5), sep) - } else { - // Everything written so far, plus what still has to follow. - used := int64(len(dst)-start) + int64(len(closingQuote)) + int64(len(r.dia.eol)) - dst = appendFiller(dst, want-used, sep) + r.scratch = appendPhrase(r.scratch[:0], rng, 3+rng.IntN(5), + r.dia.sep, r.dia.quotes.separatorsInDescription) + return r.closeRow(dst, r.scratch) + } + + // What is left for the description AND its quotes together. Which of the + // two it is comes out of fill below. + r.scratch = r.fill(r.scratch[:0], want-used-int64(len(r.dia.eol))) + return r.closeRow(dst, r.scratch) +} + +// fill builds the description of the closing row so the row lands on exactly +// the length it was asked for. +// +// room is the description and its quotes together, and how it divides between +// them is the whole of this function. With "all" the quotes are certain. With +// "none" there are none. With "minimal" it depends on the description itself, +// so the quoted length is built first and MEASURED: if it carries the +// separator the quotes are earned and that is the answer, and if it does not, +// the description is built to the full room with the separator withheld, which +// leaves nothing for a quote to be needed for. +// +// Measured on 2026-09-03: the filler first carries a separator at 30 B of +// description, so the second branch is reached by the two lengths either side +// of that. It is a narrow band and it is the only place the two halves of this +// setting could have disagreed. +func (r *rows) fill(dst []byte, room int64) []byte { + q, sep := r.dia.quotes, r.dia.sep + + if q.everyField { + return appendFiller(dst, room-2, sep, true) + } + if q.separatorsInDescription && room >= 2 { + dst = appendFiller(dst, room-2, sep, true) + if q.wraps(dst, sep) { + return dst + } + dst = dst[:0] } + return appendFiller(dst, room, sep, false) +} - dst = append(dst, closingQuote...) +// closeRow puts the description into the row and ends it. +// +// It asks the setting whether these bytes carry quotes, and fill above asked +// the same question to work the length out - one question, one answer, so the +// arithmetic and the bytes cannot part company. +func (r *rows) closeRow(dst, description []byte) []byte { + if r.dia.quotes.wraps(description, r.dia.sep) { + dst = append(dst, quoteMark) + dst = append(dst, description...) + dst = append(dst, quoteMark) + } else { + dst = append(dst, description...) + } return append(dst, r.dia.eol...) } @@ -268,10 +348,13 @@ func (r *rows) append(dst []byte, rng *rand.Rand, want int64) []byte { // needs no quoting, so a description that kept dropping commas would leave a // semicolon file never exercising the quoted path at all - the file would be // the right size, parse everywhere, and quietly test less than the comma one. -func appendPhrase(dst []byte, rng *rand.Rand, n int, sep byte) []byte { +// +// carries is false only under quote_style none, where an unquoted field cannot +// hold a separator without ending early. +func appendPhrase(dst []byte, rng *rand.Rand, n int, sep byte, carries bool) []byte { for i := 0; i < n; i++ { if i > 0 { - if i%3 == 0 { + if carries && i%3 == 0 { dst = append(dst, sep) } dst = append(dst, ' ') @@ -291,11 +374,12 @@ func appendPhrase(dst []byte, rng *rand.Rand, n int, sep byte) []byte { // A separator every fourth word, unlike every other format here, and on // purpose: the description is a quoted field, so the padding is what makes a // long file keep exercising the quoting rather than turning into plain words. -// It follows the dialect for the reason appendPhrase gives. -func appendFiller(dst []byte, n int64, sep byte) []byte { +// It follows the dialect for the reason appendPhrase gives, and it withholds +// the separator for the reason appendPhrase gives too. +func appendFiller(dst []byte, n int64, sep byte, carries bool) []byte { both := string(sep) + " " return core.AppendFiller(dst, words, n, func(i int) string { - if i%4 == 0 { + if carries && i%4 == 0 { return both } return " " diff --git a/internal/format/csvfile/dialect.go b/internal/format/csvfile/dialect.go index 259517e..362aa84 100644 --- a/internal/format/csvfile/dialect.go +++ b/internal/format/csvfile/dialect.go @@ -1,17 +1,20 @@ -// The dialect: the three ways a CSV file differs before its contents do. +// The dialect: the four ways a CSV file differs before its contents do. // // A tester does not usually get to choose the CSV they are handed. A European // export separates with a semicolon, anything written on Windows ends its rows -// with CRLF, and a feed dumped straight out of a database has no header row at -// all. All three parse as CSV and all three break readers that assumed the -// other thing, which is why they are settings rather than a fixed shape. +// with CRLF, a feed dumped straight out of a database has no header row at +// all, and a file written by a spreadsheet may quote every field or none. All +// of them parse as CSV and all of them break readers that assumed the other +// thing, which is why they are settings rather than a fixed shape. // // Every value here leaves the file exactly the size that was asked for. What -// moves is the minimum: a CRLF row is a byte longer than an LF one, and a file -// with no header has one fewer line to pay for. +// moves is the minimum: a CRLF row is a byte longer than an LF one, a file +// with no header has one fewer line to pay for, and a file that quotes every +// field pays two bytes for each of them. package csvfile import ( + "bytes" "strings" "github.com/donislawdev/TestingFilesGenerator/internal/format" @@ -22,6 +25,7 @@ const ( Delimiter = "delimiter" LineEnding = "line_ending" Header = "header" + QuoteStyle = "quote_style" ) // delimiters are the separators offered, by name rather than by character. @@ -54,7 +58,83 @@ var lineEndings = map[string]string{ // format offers it. var lineEndingIDs = []string{"lf", "crlf"} -// dialect is the settled form of the three settings: the names for the +// quoting is the settled form of quote_style: what it does rather than what it +// is called. +// +// Three values and two questions, and the two are not independent. Whether the +// plain fields carry quotes is one. Whether the description may carry the +// separator is the other, and "none" has to answer no - an unquoted field +// cannot hold a separator without ending early. So this setting changes the +// CONTENT of the file and not only the punctuation around it, which is the one +// thing about it that is not obvious from its name. +type quoting struct { + id string + + // everyField says whether the plain fields and the header names carry + // quotes, which only "all" does. None of them ever needs quoting on its + // own account: they are digits, a word, a word and a domain, an amount and + // a fixed date. + everyField bool + + // separatorsInDescription says whether the description may carry the + // separator, which is the whole reason that column was ever quoted. + separatorsInDescription bool +} + +var quoteStyles = map[string]quoting{ + "minimal": {id: "minimal", separatorsInDescription: true}, + "all": {id: "all", everyField: true, separatorsInDescription: true}, + "none": {id: "none"}, +} + +// quoteStyleIDs is the closed set, in the vocabulary RFC 4180 readers already +// use. A fourth value outside it was considered and rejected on 2026-09-02: +// see the CSV card in docs/MVP-FORMATS.md. +var quoteStyleIDs = []string{"minimal", "all", "none"} + +// wraps says whether a description of these bytes carries quotes. +// +// It reads the bytes rather than working the answer out beside them. With +// "minimal" the question is whether this particular description carries the +// separator, and the alternative is arithmetic over the word count and the +// schedule that puts separators in - two things that would have to keep +// agreeing with each other for as long as this format exists. +// +// This is the only place that question is answered. The arithmetic that sizes +// the closing row asks it too, so the length and the bytes cannot disagree. +func (q quoting) wraps(description []byte, sep byte) bool { + switch { + case q.everyField: + return true + case !q.separatorsInDescription: + return false + default: + return bytes.IndexByte(description, sep) >= 0 + } +} + +// quoteBytes is what the quotes cost in the SHORTEST row, which is the row the +// floor is made of. +// +// That row has an empty description, and an empty field carries no separator, +// so minimal leaves it bare and pays nothing. Only "all" pays, and it pays for +// every column. +func (q quoting) quoteBytes() int { + if !q.everyField { + return 0 + } + return 2 * len(columnNames) +} + +// mark writes the quote that wraps a plain field, which only "all" has. +func (q quoting) mark(dst []byte) []byte { + if !q.everyField { + return dst + } + return append(dst, quoteMark) +} + +// dialect is the settled form of the four settings: the names for the // manifest, and the bytes for the writer. type dialect struct { delimiterID string @@ -64,6 +144,8 @@ type dialect struct { eol string header bool + + quotes quoting } func defaultDialect() dialect { @@ -73,6 +155,7 @@ func defaultDialect() dialect { lineEndingID: "lf", eol: "\n", header: true, + quotes: quoteStyles["minimal"], } } @@ -119,6 +202,15 @@ func parseDialect(props map[string]string) (dialect, error) { } } + if v, ok := props[QuoteStyle]; ok && v != "" { + q, known := quoteStyles[v] + if !known { + return dialect{}, badValue(QuoteStyle, v, + "it has to be "+strings.Join(quoteStyleIDs, ", ")) + } + d.quotes = q + } + return d, nil } @@ -136,8 +228,19 @@ var columnNames = []string{"id", "name", "email", "amount", "created", "descript // That disagreement is the whole defect this format would have: a file whose // header says one thing and whose rows do another still has the right size and // still ends every line properly. +// It follows quote_style too. "all" means every field in the file, and the +// header names are fields - a writer set to quote everything quotes them as +// well, so a header left bare would be the one row disagreeing with the +// setting that produced it. func (d dialect) headerLine() string { - return strings.Join(columnNames, string(d.sep)) + d.eol + names := columnNames + if d.quotes.everyField { + names = make([]string, 0, len(columnNames)) + for _, column := range columnNames { + names = append(names, string(quoteMark)+column+string(quoteMark)) + } + } + return strings.Join(names, string(d.sep)) + d.eol } // headerBytes is what the header costs, which is nothing when there is none. @@ -168,5 +271,10 @@ func properties() []format.Property { Default: "true", Detail: "Whether the first row names the columns. Turn it off for a table dumped straight out of a database.", }, + { + Name: QuoteStyle, Kind: format.PropertyChoice, + Choices: quoteStyleIDs, Default: "minimal", + Detail: "Which fields carry quotes. With none the description stops carrying separators as well, because an unquoted field cannot hold one.", + }, } } diff --git a/internal/guard/csvdialect_test.go b/internal/guard/csvdialect_test.go index a944e86..d88298a 100644 --- a/internal/guard/csvdialect_test.go +++ b/internal/guard/csvdialect_test.go @@ -16,28 +16,33 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/oracle" ) -// csvDialects is every combination the registry offers, built from the -// declaration rather than written out. A list copied into a guard stops +// csvChoices is the closed set the registry offers for one setting, built from +// the declaration rather than written out. A list copied into a guard stops // describing the thing it guards the moment somebody adds a value. -func csvDialects(t *testing.T) (delims, endings []string) { +func csvChoices(t *testing.T, name string) []string { t.Helper() d, err := format.Get("csv") if err != nil { t.Fatal(err) } for _, p := range d.Properties { - switch p.Name { - case "delimiter": - delims = p.Choices - case "line_ending": - endings = p.Choices + if p.Name != name { + continue } + if len(p.Choices) == 0 { + t.Fatalf("csv declares %s with no values at all, so this guard would walk nothing", name) + } + return p.Choices } - if len(delims) == 0 || len(endings) == 0 { - t.Fatalf("csv declares %d separators and %d row endings, so this guard would walk nothing", - len(delims), len(endings)) - } - return delims, endings + t.Fatalf("csv declares no setting called %s, so this guard would walk nothing", name) + return nil +} + +// csvDialects is every axis this file walks. Header is not among them because +// it has two values and no declaration to read them from - it is a bool. +func csvDialects(t *testing.T) (delims, endings, styles []string) { + t.Helper() + return csvChoices(t, "delimiter"), csvChoices(t, "line_ending"), csvChoices(t, "quote_style") } // writeCSV produces one file and hands back its bytes, failing loudly rather @@ -81,11 +86,11 @@ func csvFloor(t *testing.T, props map[string]string) int64 { // The dialect asked for is the dialect in the file, and it moves the floor. // -// Three settings, sixteen combinations, and each of them is a file somebody is -// really handed: a European export separates with a semicolon, anything written -// on Windows ends its rows with CRLF, and a feed dumped out of a database has -// no header. All three parse as CSV and all three break a reader that assumed -// the other thing. +// Four settings, forty eight combinations, and each of them is a file somebody +// is really handed: a European export separates with a semicolon, anything +// written on Windows ends its rows with CRLF, a feed dumped out of a database +// has no header, and a spreadsheet may quote every field or none. All of them +// parse as CSV and all of them break a reader that assumed the other thing. // // What could be wrong here divides in two, and the halves fail differently. // @@ -99,110 +104,191 @@ func csvFloor(t *testing.T, props map[string]string) int64 { // direction for CRLF, which no size or determinism guard would ever see. That // is asked by taking the floor the format announces and the byte below it. func TestTheCSVDialectIsInTheFileAndMovesTheFloor(t *testing.T) { - delims, endings := csvDialects(t) + delims, endings, styles := csvDialects(t) seen := map[int64]bool{} for _, delim := range delims { for _, eol := range endings { for _, header := range []string{"true", "false"} { - name := delim + "/" + eol + "/header=" + header - t.Run(name, func(t *testing.T) { - props := map[string]string{ - "delimiter": delim, "line_ending": eol, "header": header, - } + for _, style := range styles { + name := delim + "/" + eol + "/header=" + header + "/" + style + t.Run(name, func(t *testing.T) { + csvDialectCase(t, seen, delim, eol, header, style) + }) + } + } + } + } - n := csvFloor(t, props) - seen[n] = true - d, err := format.Get("csv") - if err != nil { - t.Fatal(err) - } - if _, err := d.Generator.Plan(format.Request{Bytes: n, Seed: 1, Label: true, - Properties: props}); err != nil { - t.Errorf("announces %d B as its floor and then refuses it: %v", n, err) - } - if _, err := d.Generator.Plan(format.Request{Bytes: n - 1, Seed: 1, Label: true, - Properties: props}); err == nil { - t.Errorf("took %d B, one below the %d B it calls its floor", n-1, n) - } + // The floor is not one number wearing forty eight hats, and this is the + // only thing here that can say so. + // + // A floor worked out wrong is invisible everywhere else, and that is worth + // spelling out because it is not obvious. Shortest is the worst DRAW - a + // nineteen digit row number and the longest word twice - so a real closing + // row is some forty bytes under it. A floor that is one or twelve bytes + // too low still produces files of exactly the right size at every seed and + // passes every other check on this page. + // + // Eight is today's measurement, not a looser number: twelve combinations of + // the three settings that move the floor, and minimal and none share their + // four, because the row a floor is made of has an empty description and an + // empty field carries no separator to need a quote for. Measured with the + // binary on 2026-09-03 - 115, 117, 74, 75, 139, 141, 86, 87. It was written + // as six first, which is the count that would survive a build where CRLF + // stopped costing its second byte, and two mutations walked straight + // through it. + if len(seen) < 8 { + t.Errorf("forty eight dialects produced %d distinct floors and the axes make 8. A header is a whole "+ + "line, a CRLF row is a byte longer than an LF one and quoting every field costs two bytes a "+ + "column, so a floor that did not move was worked out for one dialect and handed to the rest.", + len(seen)) + } +} - // Exact to the byte, at seeds rather than at sizes somebody - // picked - the closing row is stretched to reach the length - // and its arithmetic is what the separator and the ending - // both feed into. - for seed := uint64(1); seed <= 6; seed++ { - for _, size := range []int64{n, n + 1, 1000, 8192} { - if size < n { - continue - } - writeCSV(t, size, seed, props) // fails inside on a miss - } - } +// csvDialectCase is one combination. It lives outside the loops above so the +// body is readable at the depth the code shape guard asks for. +func csvDialectCase(t *testing.T, seen map[int64]bool, delim, eol, header, style string) { + t.Helper() + props := map[string]string{ + "delimiter": delim, "line_ending": eol, "header": header, "quote_style": style, + } + n := csvFloor(t, props) + seen[n] = true + d, err := format.Get("csv") + if err != nil { + t.Fatal(err) + } + if _, err := d.Generator.Plan(format.Request{Bytes: n, Seed: 1, Label: true, + Properties: props}); err != nil { + t.Errorf("announces %d B as its floor and then refuses it: %v", n, err) + } + if _, err := d.Generator.Plan(format.Request{Bytes: n - 1, Seed: 1, Label: true, + Properties: props}); err == nil { + t.Errorf("took %d B, one below the %d B it calls its floor", n-1, n) + } - body := writeCSV(t, 8192, 9, props) - wantEOL := map[string]string{"lf": "\n", "crlf": "\r\n"}[eol] - if !bytes.HasSuffix(body, []byte(wantEOL)) { - t.Errorf("the file does not end with %s, so the last row is unterminated", eol) - } - if eol == "lf" && bytes.Contains(body, []byte("\r")) { - t.Errorf("an lf file carries a carriage return, so some readers will see a different table") - } + // Exact to the byte, at seeds rather than at sizes somebody + // picked - the closing row is stretched to reach the length + // and its arithmetic is what the separator and the ending + // both feed into. + for seed := uint64(1); seed <= 6; seed++ { + for _, size := range []int64{n, n + 1, 1000, 8192} { + if size < n { + continue + } + writeCSV(t, size, seed, props) // fails inside on a miss + } + } - sep := map[string]string{ - "comma": ",", "semicolon": ";", "tab": "\t", "pipe": "|", - }[delim] - if sep == "" { - t.Fatalf("the registry offers %q and this guard does not know what it separates with. "+ - "Add it rather than deleting this check - a value nobody described is a value nobody verified.", delim) - } - lines := strings.Split(strings.TrimSuffix(string(body), wantEOL), wantEOL) - if len(lines) < 3 { - t.Fatalf("only %d rows, too few to say anything", len(lines)) - } - for i, line := range lines[:3] { - if !strings.Contains(line, sep) { - t.Errorf("row %d carries no %s, so the setting was stored and the writer ignored it: %s", - i+1, delim, line) - } - } + body := writeCSV(t, 8192, 9, props) + wantEOL := map[string]string{"lf": "\n", "crlf": "\r\n"}[eol] + if !bytes.HasSuffix(body, []byte(wantEOL)) { + t.Errorf("the file does not end with %s, so the last row is unterminated", eol) + } + if eol == "lf" && bytes.Contains(body, []byte("\r")) { + t.Errorf("an lf file carries a carriage return, so some readers will see a different table") + } + + sep := map[string]string{ + "comma": ",", "semicolon": ";", "tab": "\t", "pipe": "|", + }[delim] + if sep == "" { + t.Fatalf("the registry offers %q and this guard does not know what it separates with. "+ + "Add it rather than deleting this check - a value nobody described is a value nobody verified.", delim) + } + lines := strings.Split(strings.TrimSuffix(string(body), wantEOL), wantEOL) + if len(lines) < 3 { + t.Fatalf("only %d rows, too few to say anything", len(lines)) + } + for i, line := range lines[:3] { + if !strings.Contains(line, sep) { + t.Errorf("row %d carries no %s, so the setting was stored and the writer ignored it: %s", + i+1, delim, line) + } + } - hasHeader := strings.HasPrefix(lines[0], "id"+sep) - if hasHeader != (header == "true") { - t.Errorf("header=%s and the first row is %q", header, lines[0]) - } + // The header names the first column, and under quote_style + // all it does so in quotes - because the header row is made + // of fields like any other, and a writer told to quote + // everything quotes those too. + firstColumn := "id" + sep + if style == "all" { + firstColumn = `"id"` + sep + } + hasHeader := strings.HasPrefix(lines[0], firstColumn) + if hasHeader != (header == "true") { + t.Errorf("header=%s under quote_style %s and the first row is %q", header, style, lines[0]) + } - // The quoted field carries the separator, which is the only - // reason the column is quoted at all. - // - // This is the half a reader of the code would not think to - // ask. The description is padding, and padding that dropped - // commas whatever the dialect would leave a semicolon file - // looking perfect - right size, right separators between the - // fields, every row the same width - while quietly never - // exercising the quoted path that the separator setting - // exists to test. Nothing else here would see it. - // - // Asked by counting: a row has five separators between its - // six fields, so any row carrying more than five has them - // inside the quotes. - padded := lines[len(lines)-1] - if n := strings.Count(padded, sep); n <= 5 { - t.Errorf("the closing row carries %d %s and six fields need five of them, "+ - "so nothing sits inside the quotes and this dialect never exercises quoting: %.120s", - n, delim, padded) - } - }) - } + // No dialect leaks another dialect's separator into the file. + // + // This is the half a reader of the code would not think to ask. The + // description is padding, and padding that dropped commas whatever the + // dialect was asked for would leave a semicolon file looking perfect - + // right size, right separators between the fields, every row the same + // width - while quietly never exercising the quoted path that the + // separator setting exists to test. Nothing else here would see it, and + // asking it this way needs no threshold and no lucky row. + for other, ch := range map[string]string{"comma": ",", "semicolon": ";", "tab": "\t", "pipe": "|"} { + if other == delim || !strings.Contains(string(body), ch) { + continue + } + t.Errorf("a %s file carries a %s, so something in it is separating with a character "+ + "this dialect never asked for", delim, other) + } + + csvPaddingCarriesTheSeparator(t, props, sep, delim, style, wantEOL) + + if style == "none" && bytes.Contains(body, []byte(`"`)) { + t.Error("quote_style none produced a file with a quote in it") + } + if style == "all" && !bytes.HasPrefix(body, []byte(`"`)) { + t.Errorf("quote_style all left the first field bare: %.60s", lines[0]) + } +} + +// csvPaddingCarriesTheSeparator asks whether the padded row puts the separator +// inside the quotes, which is the only reason that column is ever quoted. +// +// Asked by counting: a row has five separators between its six fields, so any +// row carrying more than five has them inside the quotes. +// +// Over a BAND of sizes rather than one, and that is the whole reason this is a +// function of its own. The closing row is whatever length was left over, and a +// short one carries no separator at all - which is legal, and which made the +// first version of this check fail on quote_style all while the build was +// right. Measured 2026-09-03: the filler first carries a separator at 30 B of +// description, and a closing description runs anywhere from empty to about +// four times that. One size proves nothing either way. So the band is walked +// and the count is asserted, rather than one row being assumed to be long. +func csvPaddingCarriesTheSeparator(t *testing.T, props map[string]string, sep, delim, style, eol string) { + t.Helper() + const band = 24 + carried, widest := 0, 0 + for size := int64(8192); size < 8192+band; size++ { + body := string(writeCSV(t, size, 9, props)) + rows := strings.Split(strings.TrimSuffix(body, eol), eol) + last := rows[len(rows)-1] + if inside := strings.Count(last, sep) - 5; inside > 0 { + carried++ + } + if len(last) > widest { + widest = len(last) } } - // The floor is not one number wearing sixteen hats. Without this the whole - // test above would still pass on a build that ignored the dialect when - // working the floor out. - if len(seen) < 4 { - t.Errorf("sixteen dialects produced %d distinct floors. A header is a whole line and a CRLF row "+ - "is a byte longer than an LF one, so a floor that did not move was worked out for one dialect "+ - "and handed to the rest.", len(seen)) + if style == "none" { + if carried != 0 { + t.Errorf("%d of %d closing rows carry a %s beyond the five that separate the fields, and "+ + "quote_style none has no quotes to hide one in", carried, band, delim) + } + return + } + if carried == 0 { + t.Errorf("not one of %d closing rows carries a %s inside its description - the widest of them was "+ + "%d B, so either the padding stopped emitting the separator or this guard never reached a row "+ + "long enough to hold one", band, delim, widest) } } @@ -220,7 +306,7 @@ func TestTheCSVDialectIsInTheFileAndMovesTheFloor(t *testing.T) { // telling it another has to come back refused, or every pass above is worth // nothing. func TestEveryCSVDialectIsWellFormed(t *testing.T) { - delims, endings := csvDialects(t) + delims, endings, styles := csvDialects(t) dir := t.TempDir() check := func(t *testing.T, body []byte, name string, settings ...string) oracle.Result { @@ -236,22 +322,25 @@ func TestEveryCSVDialectIsWellFormed(t *testing.T) { for _, delim := range delims { for _, eol := range endings { for _, header := range []string{"true", "false"} { - name := delim + "_" + eol + "_" + header - t.Run(name, func(t *testing.T) { - props := map[string]string{ - "delimiter": delim, "line_ending": eol, "header": header, - } - body := writeCSV(t, 8192, 9, props) - res := check(t, body, name, - "delimiter="+delim, "line_ending="+eol, "header="+header) - if !res.Available { - t.Skip("the structural check needs python") - } - ran++ - if res.Err != nil { - t.Errorf("%s is not well formed: %v", name, res.Err) - } - }) + for _, style := range styles { + name := delim + "_" + eol + "_" + header + "_" + style + t.Run(name, func(t *testing.T) { + props := map[string]string{ + "delimiter": delim, "line_ending": eol, + "header": header, "quote_style": style, + } + body := writeCSV(t, 8192, 9, props) + res := check(t, body, name, "delimiter="+delim, "line_ending="+eol, + "header="+header, "quote_style="+style) + if !res.Available { + t.Skip("the structural check needs python") + } + ran++ + if res.Err != nil { + t.Errorf("%s is not well formed: %v", name, res.Err) + } + }) + } } } } @@ -259,18 +348,33 @@ func TestEveryCSVDialectIsWellFormed(t *testing.T) { if ran == 0 { t.Skip("the structural check never ran, so nothing here was judged") } - if ran != len(delims)*len(endings)*2 { - t.Errorf("%d of %d dialects reached the checker", ran, len(delims)*len(endings)*2) + if want := len(delims) * len(endings) * len(styles) * 2; ran != want { + t.Errorf("%d of %d dialects reached the checker", ran, want) } // The checker is told, so it has to disagree when it is told wrong. - body := writeCSV(t, 8192, 4, map[string]string{"delimiter": "comma", "line_ending": "lf"}) - for _, wrong := range []struct{ what, setting string }{ - {"a comma file called semicolon", "delimiter=semicolon"}, - {"a comma file called tab", "delimiter=tab"}, - {"an lf file called crlf", "line_ending=crlf"}, + // + // Quoting needs both directions here and the dialect above does not, + // because quoting is the one axis where every value produces a well formed + // file. A file that quotes nothing and a file that quotes everything both + // parse, so nothing about the table gives the style away - if the checker + // were a rubber stamp on this axis it would be a rubber stamp silently. + plain := writeCSV(t, 8192, 4, map[string]string{"delimiter": "comma", "line_ending": "lf"}) + everything := writeCSV(t, 8192, 4, map[string]string{"quote_style": "all"}) + nothing := writeCSV(t, 8192, 4, map[string]string{"quote_style": "none"}) + for _, wrong := range []struct { + what, setting string + body []byte + }{ + {"a comma file called semicolon", "delimiter=semicolon", plain}, + {"a comma file called tab", "delimiter=tab", plain}, + {"an lf file called crlf", "line_ending=crlf", plain}, + {"a minimal file called all", "quote_style=all", plain}, + {"an all file called minimal", "quote_style=minimal", everything}, + {"an all file called none", "quote_style=none", everything}, + {"a none file called all", "quote_style=all", nothing}, } { - res := check(t, body, "wrong_"+strings.ReplaceAll(wrong.setting, "=", "_"), wrong.setting) + res := check(t, wrong.body, "wrong_"+strings.ReplaceAll(wrong.setting, "=", "_"), wrong.setting) if !res.Available { t.Skip("the structural check needs python") } @@ -298,11 +402,14 @@ func TestTheCSVManifestRecordsTheDialectAsFacts(t *testing.T) { sep string eol string head bool + style string }{ - {map[string]string{}, ",", "lf", true}, - {map[string]string{"delimiter": "semicolon"}, ";", "lf", true}, - {map[string]string{"delimiter": "tab", "line_ending": "crlf"}, "\t", "crlf", true}, - {map[string]string{"delimiter": "pipe", "header": "false"}, "|", "lf", false}, + {map[string]string{}, ",", "lf", true, "minimal"}, + {map[string]string{"delimiter": "semicolon"}, ";", "lf", true, "minimal"}, + {map[string]string{"delimiter": "tab", "line_ending": "crlf"}, "\t", "crlf", true, "minimal"}, + {map[string]string{"delimiter": "pipe", "header": "false"}, "|", "lf", false, "minimal"}, + {map[string]string{"quote_style": "all"}, ",", "lf", true, "all"}, + {map[string]string{"quote_style": "none", "delimiter": "tab"}, "\t", "lf", true, "none"}, } for _, c := range cases { t.Run(fmt.Sprint(c.props), func(t *testing.T) { @@ -319,6 +426,12 @@ func TestTheCSVManifestRecordsTheDialectAsFacts(t *testing.T) { if got := p.Properties["header"]; got != c.head { t.Errorf("header is %v, wanted %v", got, c.head) } + // The word rather than a count of quotes, because the recipe and + // the manifest name this one the same way. There is no second + // spelling of it the way there is for the separator. + if got := p.Properties["quote_style"]; got != c.style { + t.Errorf("quote_style is %v, wanted %q", got, c.style) + } // And it stays JSON a script can read, rather than something that // only looks right in a Go printout. if _, err := json.Marshal(p.Properties); err != nil { diff --git a/internal/guard/generatorbytes_test.go b/internal/guard/generatorbytes_test.go index 27988f9..6621343 100644 --- a/internal/guard/generatorbytes_test.go +++ b/internal/guard/generatorbytes_test.go @@ -216,7 +216,18 @@ func goldenCases() map[string]engine.Target { // longer address, and a terminator of two bytes instead of one. "log_nginx_v6_crlf": {ID: "g", Format: "log", Sizes: engine.Uniform(1, 8193), Label: true, Properties: map[string]string{"entry_format": "nginx", "ip_version": "v6", "line_ending": "crlf"}}, - "csv_8kib": {ID: "g", Format: "csv", Sizes: engine.Uniform(1, 8192), Label: true}, + "csv_8kib": {ID: "g", Format: "csv", Sizes: engine.Uniform(1, 8192), Label: true}, + + // The two quote styles that are not the default. Each one is a + // different file rather than the same file punctuated differently: + // "all" wraps every field including the header, and "none" makes the + // description stop carrying the separator, because an unquoted field + // cannot hold one. Both change the floor as well as the bytes, which is + // why neither is covered by the case above. + "csv_8kib_quote_all": {ID: "g", Format: "csv", Sizes: engine.Uniform(1, 8192), Label: true, + Properties: map[string]string{"quote_style": "all"}}, + "csv_8kib_quote_none": {ID: "g", Format: "csv", Sizes: engine.Uniform(1, 8192), Label: true, + Properties: map[string]string{"quote_style": "none"}}, "json_8kib": {ID: "g", Format: "json", Sizes: engine.Uniform(1, 8192), Label: true}, "xml_8kib": {ID: "g", Format: "xml", Sizes: engine.Uniform(1, 8192), Label: true}, "html_8kib": {ID: "g", Format: "html", Sizes: engine.Uniform(1, 8192), Label: true}, diff --git a/internal/guard/parity_test.go b/internal/guard/parity_test.go index ba55cb4..94618ff 100644 --- a/internal/guard/parity_test.go +++ b/internal/guard/parity_test.go @@ -102,6 +102,7 @@ var reachableFromTheWindow = []string{ "property:csv.delimiter", "property:csv.header", "property:csv.line_ending", + "property:csv.quote_style", "property:docx.paragraphs", "property:pptx.slides", "property:xlsx.columns", diff --git a/internal/guard/testdata/generator-golden.json b/internal/guard/testdata/generator-golden.json index 9b6e562..568497f 100644 --- a/internal/guard/testdata/generator-golden.json +++ b/internal/guard/testdata/generator-golden.json @@ -30,7 +30,7 @@ }, "csv_8kib": { "bytes": 8192, - "sha256": "9c27e8266485816183ac867c69fe3ae797aeab185a2f5b38c99acdc957076998" + "sha256": "0fe75bae3e3b557c2047ca89cca103389dffd8e8b96c65b8c0cf2658605adb28" }, "docx_32kib": { "bytes": 32768, @@ -243,6 +243,14 @@ "zip_with_three_pdfs": { "bytes": 65536, "sha256": "2f45c46662ab65d2f598d65446ba0476067fb5c6297dc497d580a0fcd1752fbe" + }, + "csv_8kib_quote_all": { + "bytes": 8192, + "sha256": "6d863b984384d879fd9026f07855d816e70538cf208819b03cd734a2a682f739" + }, + "csv_8kib_quote_none": { + "bytes": 8192, + "sha256": "d596eccde18c924fe0245b21523a6fc47c90f2d322131e0b5bcb03dc8a5d8b93" } }, "remeasured": [ @@ -287,6 +295,15 @@ { "on": "2026-09-01", "why": "The project moved from Go 1.26.7 to Go 1.27.0, and Go 1.27 changed compress/flate. Every format here that puts bytes through deflate produces different ones: docx, pptx and xlsx compress their parts, png compresses its image data, ico when it holds a png, and targz through gzip. Sizes are unchanged - every one of these files is the length it was, and the same sizes are reachable. At compression level zero the change is framing rather than algorithm: the block that closes the stream went from five bytes to two. Above level zero the algorithm itself moved, so the difference is larger and not constant. zip is absent from this list because its default is store, which puts nothing through deflate - ask for compression and it moves too. The tar.gz arithmetic that broke on this now MEASURES the framing rather than carrying it as a constant, so a later Go release moves the bytes again but does not stop the format from being produced." + }, + { + "on": "2026-09-03", + "why": "csv learned quote_style, and its default value minimal changes the file. A description is quoted only when it carries the separator now, and the phrase builder draws three to seven words while dropping a separator every third one - so a three word description carries none and loses its quotes. Measured on a 4 kB table at seed 7 before the change: 9 of 44 rows. Sizes are unchanged and every size that worked still works, but the floor moved from 117 B to 115 B, because the shortest row has an empty description and an empty field carries no separator. The two cases beside it are the other two values, pinned from the first day they exist rather than after somebody notices they were not: all wraps every field including the header and floors at 139 B, none wraps nothing and makes the description stop carrying the separator, because an unquoted field cannot hold one. The owner decided this on 2026-09-02 and again on 2026-09-03: the release this belongs to closes with a major bump either way, so a clean RFC 4180 vocabulary costs nothing here and a fourth value outside it would have been carried forever.", + "files": [ + "csv_8kib", + "csv_8kib_quote_all", + "csv_8kib_quote_none" + ] } ] } diff --git a/internal/guard/textformats_test.go b/internal/guard/textformats_test.go index 76f7916..97cd86f 100644 --- a/internal/guard/textformats_test.go +++ b/internal/guard/textformats_test.go @@ -160,7 +160,7 @@ const ( const minRecords = densitySize / maxValueBytes func TestEveryRowOfACSVHasTheSameColumns(t *testing.T) { - for _, size := range []int64{117, 118, 512, 4097, 32769, densitySize} { + for _, size := range []int64{115, 116, 512, 4097, 32769, densitySize} { t.Run(sizeText(size), func(t *testing.T) { body := generateBytes(t, "csv", size) if int64(len(body)) != size { diff --git a/internal/oracle/strict.py b/internal/oracle/strict.py index ac44718..dd39192 100644 --- a/internal/oracle/strict.py +++ b/internal/oracle/strict.py @@ -289,6 +289,12 @@ def check_csv(data, settings=None): file uses the separator that was ordered is a question for a guard reading the manifest, not for this. Told, it can still catch the defect that matters here: a header and its rows disagreeing about the separator. + + Quoting is judged as well as parsed, and that is a separate job from + reading the table. Every one of the three styles parses - a file that + quoted nothing and a file that quoted everything are both well formed RFC + 4180 - so the shape of the table says nothing at all about whether the + style that was ordered is the style in the file. """ FIELD_CEILING = 131072 @@ -296,6 +302,9 @@ def check_csv(data, settings=None): sep = CSV_DELIMITERS[settings.get("delimiter", "comma")] eol = CSV_LINE_ENDINGS[settings.get("line_ending", "lf")] has_header = settings.get("header", "true") == "true" + style = settings.get("quote_style", "minimal") + if style not in ("minimal", "all", "none"): + fail(f"quote_style {style!r} is not one of minimal, all, none") try: text = data.decode("utf-8") @@ -306,7 +315,11 @@ def check_csv(data, settings=None): fail(f"the file does not end with {settings.get('line_ending', 'lf')}, " "so the last row is unterminated") - rows, field, row, quoted, i = [], [], [], False, 0 + # Each field is kept with whether the RAW TEXT wrapped it in quotes, which + # is the only thing that survives parsing and the only thing quote_style is + # about. A parser that returned values alone could not tell the three + # styles apart at all. + rows, field, row, quoted, wrapped, i = [], [], [], False, False, 0 while i < len(text): ch = text[i] if quoted: @@ -323,13 +336,13 @@ def check_csv(data, settings=None): if ch == '"': if field: fail(f"row {len(rows) + 1} opens a quote in the middle of a field") - quoted = True + quoted, wrapped = True, True elif ch == sep: - row.append("".join(field)) - field = [] + row.append(("".join(field), wrapped)) + field, wrapped = [], False elif text.startswith(eol, i): - row.append("".join(field)) - field = [] + row.append(("".join(field), wrapped)) + field, wrapped = [], False rows.append(row) row = [] i += len(eol) @@ -362,16 +375,41 @@ def check_csv(data, settings=None): fail(f"the first row has {columns} column(s), so nothing is separated by " f"the {settings.get('delimiter', 'comma')} this file is meant to use") declares = "the header" if has_header else "the first row" + quoted_fields = 0 for number, r in enumerate(rows, start=1): if len(r) != columns: fail(f"row {number} has {len(r)} fields and {declares} declares {columns}") - for value in r: + for value, was_quoted in r: + quoted_fields += was_quoted if len(value.encode("utf-8")) > FIELD_CEILING: fail(f"row {number} has a field of {len(value.encode('utf-8'))} B - " f"the default Python reader refuses anything above {FIELD_CEILING} B") + check_quoting(style, number, value, was_quoted, sep) data = len(rows) - 1 if has_header else len(rows) - ok(f"{data} data rows, {columns} columns each") + ok(f"{data} data rows, {columns} columns each, {quoted_fields} quoted fields") + + +def check_quoting(style, number, value, was_quoted, sep): + """One field against the quote_style the file was ordered in. + + Only one direction is checkable under minimal, and that is a property of + CSV rather than a gap here: a field that carried a separator without quotes + would have been SPLIT before it reached this, so it arrives as two fields + of the wrong width and the check above catches it as a ragged row. What + cannot be caught that way is the opposite - a quote around a value that + never needed one - so that is what this asks. + """ + needs = any(ch in value for ch in (sep, '"', chr(13), chr(10))) + if style == "all" and not was_quoted: + fail(f"row {number} has the bare field {value[:40]!r} and quote_style all " + "means every field carries quotes") + if style == "none" and was_quoted: + fail(f"row {number} quotes the field {value[:40]!r} and quote_style none " + "means no field does") + if style == "minimal" and was_quoted and not needs: + fail(f"row {number} quotes the field {value[:40]!r}, which holds no separator, " + "no quote and no line break - quote_style minimal quotes only what needs it") def check_json(data): diff --git a/web/public/formats/index.html b/web/public/formats/index.html index b3f2c34..f50616d 100644 --- a/web/public/formats/index.html +++ b/web/public/formats/index.html @@ -110,7 +110,7 @@

24 file formats, every one generated at an exact size

csv .csv - 117 + 115 full python-csv @@ -355,6 +355,11 @@

Settings each format accepts

header true or false + + + quote_style + all, minimal, none + docx paragraphs diff --git a/web/public/pl/formaty/index.html b/web/public/pl/formaty/index.html index c8aea5e..8af92be 100644 --- a/web/public/pl/formaty/index.html +++ b/web/public/pl/formaty/index.html @@ -109,7 +109,7 @@

24 formatów plików, każdy generowany o dokładnym rozmiarze

csv .csv - 117 + 115 full python-csv @@ -355,6 +355,11 @@

Ustawienia, które przyjmuje każdy format

header prawda albo fałsz + + + quote_style + all, minimal, none + docx paragraphs From 5da8019f15872a749156e1b04f9b6e20db31e95b Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 3 Sep 2026 15:26:41 +0200 Subject: [PATCH 2/3] ci: state the coverage timeout, and guard the README against the registry Two things, and the first is why CI was red. The coverage gate died on a limit nobody had chosen. Go allows ten minutes per package by default while that job allows twenty for all of it, so internal/guard hit the first without coming near the second - a stack trace out of whichever test was running when the alarm went off, instead of a failure naming something. Exactly what the race job above it met on 2026-08-25 and fixed the same way. Measured on the runner rather than guessed: the step took 375 s and 457 s on two consecutive main runs, which is 22 percent of variance on code that barely moved, and the default cuts in at 600 s. This branch added eight seconds of coverage instrumented work - measured, both new CSV guards together - and tipped it. Eight seconds is not what went wrong. 457 against 600 was never a margin. The second thing is O176: the README settings table had no guard and disagreed with the registry on seven rows. log said "none" while carrying eight settings, zip and targz listed three of eight, and avif and jxl had no row at all. The site has had this guard since it was built. The one page a visitor reads first did not. Two guards rather than one, because the table turned out to be the second half of the problem. The list at the top of the README was missing jxl outright - it arrived on 2026-08-31 as the twenty fourth format and never reached that list, so the page offered twenty three while the binary shipped twenty four. The prose said "twenty two" in three places and "24" in two: one file, three different numbers about one thing. Settings are compared as SETS. Whether a row reads "width, height" or the other way round is a question about English, and a guard answering it would be refusing prose rather than catching a lie. The count spelled out in words is deliberately not guarded. That was measured and rejected on 2026-08-05 in copiednumbers_test.go: the general form raised 43 findings and most were false, because "24 formats" and "25 formats" and "150 formats" answer three different questions here. Both guards are proven by mutation - a setting renamed in the registry, and a format registered under a name the list does not carry. That matters because the neighbouring TestTheFormatDocumentAgreesWithTheRegistry sits on notProvenByMutation, and that list is only allowed to shrink. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 23 ++- README.md | 14 +- internal/guard/readmesettings_test.go | 218 ++++++++++++++++++++++++++ 3 files changed, 248 insertions(+), 7 deletions(-) create mode 100644 internal/guard/readmesettings_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 774fb4b..67f9142 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -727,10 +727,31 @@ jobs: # package, and by default Go credits coverage only to the package # under test - which reports 0.0% and makes the gate meaningless. # Measured, not assumed. + # + # The timeout is stated rather than left to Go, since 2026-09-03, and + # for the same reason the race job above states its own. Go allows ten + # minutes PER PACKAGE by default while this job allows twenty for all of + # it, so internal/guard died on a limit nobody had chosen - a stack + # trace out of whichever test was running when the alarm went off, + # instead of a failure naming something. + # + # Measured on the runner rather than guessed. This step took 375 s and + # 457 s on two consecutive main runs of 2026-09-02 and 2026-09-03, which + # is 22 percent of variance on code that barely moved between them, and + # the default cuts in at 600 s. A branch adding eight seconds of + # coverage instrumented work then timed out. Eight seconds is not what + # went wrong: 457 against 600 was never a margin, and a limit that + # decides on how busy the runner is tells you nothing about the code. + # + # Atomic counters are the cost. Every statement in every internal + # package pays one, and this package renders twenty five screens and + # generates files for twenty four formats. Eighteen minutes sits under + # the job's own ceiling on purpose, so a genuinely stuck run still fails + # as a test with output rather than as a killed job without any. run: > go test -tags "$(cat .github/build-tags)" ./... -count=1 -covermode=atomic -coverpkg=./internal/...,./cmd/... - -coverprofile=coverage.out + -coverprofile=coverage.out -timeout 18m - name: gate # The threshold lives in exactly one place, .github/coverage-threshold. diff --git a/README.md b/README.md index 62d9f12..f591081 100644 --- a/README.md +++ b/README.md @@ -76,13 +76,13 @@ reference is below it. ## 📁 Formats it generates -Twenty two, and every one is a **real file of that format** - it opens in the +Twenty four, and every one is a **real file of that format** - it opens in the software that owns it, at the exact size you asked for: | group | formats | |---|---| | 📄 **Documents** | `pdf`, `docx` (Word), `xlsx` (Excel), `pptx` (PowerPoint) | -| 🖼️ **Images** | `png`, `jpg`, `bmp`, `gif`, `ico`, `svg`, `tiff`, `webp`, `avif` | +| 🖼️ **Images** | `png`, `jpg`, `bmp`, `gif`, `ico`, `svg`, `tiff`, `webp`, `avif`, `jxl` | | 📝 **Text and markup** | `txt`, `md`, `csv`, `json`, `xml`, `html`, `log` | | 🗜️ **Archives** | `zip`, `targz` (`.tar.gz`) | | 🔊 **Audio** | `wav` | @@ -440,7 +440,7 @@ ignored quietly: `extends`, `with`, `policy`, `engine`, `defaults.fill`, ## 📁 Formats in detail -The twenty two formats are listed near the top of this file. Each is produced at an +The twenty four formats are listed near the top of this file. Each is produced at an exact size and checked against independent readers before it ships - a PNG is opened and its pixels compared, a DOCX is read back by three separate libraries, an archive is extracted. @@ -460,14 +460,16 @@ recipe. `tfg formats ` prints the allowed range or list for each: | `pdf` | `pages`, `page_size` | | `png`, `bmp`, `tiff`, `webp` | `width`, `height` | | `gif` | `width`, `height`, `frames` | -| `jpg` | `width`, `height`, `quality` | +| `avif`, `jpg`, `jxl` | `width`, `height`, `quality` | | `ico` | `width`, `height`, `embed` | | `wav` | `sample_rate`, `bit_depth`, `channels`, `content` | -| `zip`, `targz` | `entries`, `entry_format`, `entry_size` | +| `zip` | `entries`, `entry_format`, `entry_size`, `compression`, `depth`, `directory_entries`, `password`, `encryption` | +| `targz` | `entries`, `entry_format`, `entry_size`, `compression`, `depth`, `directory_entries`, `entry_mode`, `entry_owner` | | `docx` | `paragraphs` | | `xlsx` | `rows`, `columns` | | `pptx` | `slides` | | `csv` | `delimiter`, `line_ending`, `header`, `quote_style` | +| `log` | `entry_format`, `timestamps`, `rate`, `methods`, `status_mix`, `level_mix`, `ip_version`, `line_ending` | | `json`, `xml`, `html`, `md`, `txt`, `svg` | none | ``` @@ -685,7 +687,7 @@ a valid one of its format. Honest scope, because a tool that oversells itself wastes your afternoon. -**Working end to end:** twenty two formats, recipes, presets, the desktop window, +**Working end to end:** twenty four formats, recipes, presets, the desktop window, `generate`, `validate`, `verify`, `cleanup`, boundary sets, archive contents, size ranges, per format settings, manifests and every exit code above. diff --git a/internal/guard/readmesettings_test.go b/internal/guard/readmesettings_test.go new file mode 100644 index 0000000..946ebf0 --- /dev/null +++ b/internal/guard/readmesettings_test.go @@ -0,0 +1,218 @@ +package guard + +import ( + "os" + "path/filepath" + "regexp" + "sort" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" + _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" +) + +// The README tells a reader which settings each format takes, and until +// 2026-09-03 nothing asked whether that was true. It was not, on seven rows - +// O176 in docs/OBSERVATIONS.md has the measurement. +// +// The shape of the drift is worth naming, because it is not carelessness. The +// table is the last thing anybody thinks of when a setting is added: the +// registry declares it, the window draws it, tfg formats prints it, the site is +// regenerated by its own guard, and the README sits outside all of that. So it +// went stale four times in a row without one red run. log said "none" while +// carrying eight settings, zip and targz listed three of eight, and avif and +// jxl had no row at all. +// +// The site has had this guard since it was built - TestTheSiteSaysWhatTheToolSays +// renders the format table from the registry and compares it against what is +// published. This is the same promise for the one page a visitor reads first. +// +// Sets rather than sequences, deliberately. Whether a row reads "width, height" +// or "height, width" is a question about prose, and a guard that answered it +// would be refusing English rather than catching a lie. What it refuses is a +// setting named that does not exist, a setting that exists and is not named, +// and a format missing from the table altogether. +func TestTheReadmeSettingsTableAgreesWithTheRegistry(t *testing.T) { + rows := readmeSettingsTable(t) + if len(rows) == 0 { + t.Fatal("the per format settings table has no rows - this guard would pass against any README ever written") + } + + descriptors := format.All() + if len(descriptors) == 0 { + t.Fatal("no format is registered - this guard would pass without checking anything") + } + + declared := map[string][]string{} + for _, d := range descriptors { + names := make([]string, 0, len(d.Properties)) + for _, p := range d.Properties { + names = append(names, p.Name) + } + sort.Strings(names) + declared[d.ID] = names + } + + seen := map[string]string{} + for _, row := range rows { + for _, id := range row.formats { + if _, ok := declared[id]; !ok { + t.Errorf("the README settings table names the format %q and nothing registers it", id) + continue + } + if where, twice := seen[id]; twice { + t.Errorf("%s is in the settings table twice, here and in the row for %s - "+ + "two rows can disagree, and the one a reader believes is whichever they read first", id, where) + continue + } + seen[id] = strings.Join(row.formats, ", ") + + want := declared[id] + got := append([]string(nil), row.settings...) + sort.Strings(got) + if strings.Join(got, ",") != strings.Join(want, ",") { + t.Errorf("the README says %s takes %s and the registry declares %s. "+ + "Fix the table rather than this guard - a reader who is told a format has no settings "+ + "does not go and check", id, listOrNone(got), listOrNone(want)) + } + } + } + + for id := range declared { + if _, ok := seen[id]; !ok { + t.Errorf("%s is registered and the README settings table has no row for it, so its settings "+ + "are documented nowhere a reader of that page would look", id) + } + } +} + +// The list a reader meets first names every format there is. +// +// Separate from the settings table above because it is a different promise and +// it went stale on its own: jxl was added on 2026-08-31 as the twenty fourth +// format and never reached this list, so the page a visitor lands on offered +// twenty three while the binary shipped twenty four. Nothing was asking. +// +// One direction only, and that is deliberate rather than lazy. The table names +// formats that exist, so a registered format missing from it is a lie. The +// other direction - a name in the table that nothing registers - is already +// refused by the settings table below it, which every format has to appear in +// too, and asking twice here would put the same claim in two places. +// +// What this does NOT check is the COUNT in the sentence above the table, +// which is spelled out in words. That is left alone on purpose: the copied +// numbers guard measured the general form of it on 2026-08-05, raised 43 +// findings and found most of them false, because "24 formats" and "25 formats" +// and "150 formats" answer three different questions in this repository. The +// count in that one sentence stays on the reader. +func TestTheReadmeListsEveryFormatItShips(t *testing.T) { + body, err := os.ReadFile(filepath.Join(repoRoot(t), "README.md")) + if err != nil { + t.Fatalf("reading the README: %v", err) + } + const heading = "## 📁 Formats it generates" + text := string(body) + start := strings.Index(text, heading) + if start < 0 { + t.Fatalf("the README has no %q heading, so this guard has nothing to read", heading) + } + end := strings.Index(text[start+len(heading):], "\n## ") + if end < 0 { + t.Fatal("the formats section runs to the end of the README, which means the heading after it moved") + } + section := text[start : start+len(heading)+end] + + listed := map[string]bool{} + for _, name := range spansIn(section) { + listed[name] = true + } + if len(listed) == 0 { + t.Fatal("the formats section names nothing - this guard would pass against any README ever written") + } + + descriptors := format.All() + if len(descriptors) == 0 { + t.Fatal("no format is registered - this guard would pass without checking anything") + } + for _, d := range descriptors { + if !listed[d.ID] { + t.Errorf("%s is registered and the list under %q does not name it, so the first page a "+ + "visitor reads offers fewer formats than the binary ships", d.ID, heading) + } + } +} + +// listOrNone words a list the way the table does, so a failure can be compared +// against the line it is about without translating between two spellings. +func listOrNone(names []string) string { + if len(names) == 0 { + return "none" + } + return strings.Join(names, ", ") +} + +// settingsRow is one line of the table: the formats it speaks for, and what it +// says they take. +type settingsRow struct { + formats []string + settings []string +} + +var codeSpan = regexp.MustCompile("`([^`]+)`") + +// readmeSettingsTable reads the table under its own heading. +// +// Anchored on the heading rather than on "the first table with two columns", +// because the README holds several two column tables and a guard that found the +// wrong one would be green about something nobody asked. +func readmeSettingsTable(t *testing.T) []settingsRow { + t.Helper() + const heading = "### Per format settings" + + body, err := os.ReadFile(filepath.Join(repoRoot(t), "README.md")) + if err != nil { + t.Fatalf("reading the README: %v", err) + } + text := string(body) + + start := strings.Index(text, heading) + if start < 0 { + t.Fatalf("the README has no %q heading, so this guard has nothing to read. "+ + "If the section was renamed, rename it here too rather than deleting the check", heading) + } + + var rows []settingsRow + for _, line := range strings.Split(text[start+len(heading):], "\n") { + line = strings.TrimSpace(line) + if !strings.HasPrefix(line, "|") { + // The table has ended. Anything after it belongs to another + // section, so reading on would collect unrelated tables. + if len(rows) > 0 { + break + } + continue + } + cells := strings.Split(strings.Trim(line, "|"), "|") + if len(cells) != 2 { + continue + } + formats := spansIn(cells[0]) + if len(formats) == 0 { + // The header row and the dashes under it. + continue + } + rows = append(rows, settingsRow{formats: formats, settings: spansIn(cells[1])}) + } + return rows +} + +// spansIn pulls the code spans out of one cell, which is how both halves of the +// table name things. +func spansIn(cell string) []string { + var out []string + for _, m := range codeSpan.FindAllStringSubmatch(cell, -1) { + out = append(out, strings.TrimSpace(m[1])) + } + return out +} From f2134673de761d82a37ea8508fd0a279108a2a55 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 3 Sep 2026 15:50:36 +0200 Subject: [PATCH 3/3] ci: every whole tree test run states its timeout, and a guard keeps it that way Follow through on the commit before this one, which fixed the coverage gate and left three jobs sitting in exactly the same place. Measured on the green run of 2026-09-03, rather than assumed from the one job that went red: the test step takes 399 s on ubuntu, 476 s on windows and 491 s on macOS, all against Go's unstated ten minutes a package. macOS had 109 s of room. The same fleet was measured swinging from 479 s to past 600 s between two runs of one branch, so the margin was smaller than the variance on every one of them. The release workflow had the same gap, and there a timeout would read as a red tree and stop a release that was fine. Six whole tree runs now state a timeout. The two that already did are unchanged. The guard is the point of this commit rather than the flags. This is the SECOND time the project has lost a run to Go's default - the race detector met it on 2026-08-25 and the answer was a long comment beside that one job, which is why the coverage gate met it again eight days later. A diagnosis recorded at one step does not protect the next one, so the reasoning has moved out of the comments and into TestEveryWholeTreeTestRunStatesItsOwnTimeout. Proven by mutation. It reads the workflows through the YAML parser rather than as text, because a run block can be folded and the flag would then sit on a different line from the command. Only whole tree runs are asked: a targeted -run walks a handful of tests, and the fuzz step carries -fuzztime, which is its own budget. One stale claim fixed on the way. The matrix job's own comment said "the matrix runs in about a minute", which had not been true for a long time - it is eight, and the numbers are written down now instead of a word. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 23 ++++- .github/workflows/release.yml | 8 +- internal/guard/workflowtimeouts_test.go | 123 ++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 internal/guard/workflowtimeouts_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 67f9142..95a7d36 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,9 +32,12 @@ jobs: test: name: test on ${{ matrix.os }} # A hung job otherwise holds a runner until the GitHub default of six - # hours. Every number here is well above what the job takes today: the - # matrix runs in about a minute, the race detector took 148 s when it was - # measured, and fuzzing is given 5 minutes a target by its own loop. + # hours. Remeasured 2026-09-03, because the sentence here said "the matrix + # runs in about a minute" and had not been true for a long time: the test + # step alone takes 399 s on ubuntu, 476 s on windows and 491 s on macOS. + # The race detector took 148 s when it was measured and has its own job + # and its own numbers now, and fuzzing is given 5 minutes a target by its + # own loop. timeout-minutes: 20 strategy: fail-fast: false @@ -244,7 +247,19 @@ jobs: shell: bash - name: test - run: go test -tags "$(cat .github/build-tags)" ./... -count=1 + # The timeout is stated for the reason the race job and the coverage + # gate both state theirs: Go allows ten minutes PER PACKAGE by default, + # this job allows twenty for all of it, and internal/guard is one + # package holding almost every test there is. A run that went past the + # first without approaching the second would die as a stack trace out + # of whichever test happened to be running, which is what the coverage + # gate did on 2026-09-03. + # + # Measured that day, on the run that caught it: 399 s on ubuntu, 476 s + # on windows, 491 s on macOS. macOS therefore had 109 s of room under a + # limit nobody had chosen, and the same fleet was measured swinging by + # more than 25 percent between two runs of one branch. + run: go test -tags "$(cat .github/build-tags)" ./... -count=1 -timeout 18m - name: build the command line binary run: go build -tags "$(cat .github/build-tags)" ./cmd/tfg diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4f64887..704c274 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -51,7 +51,13 @@ jobs: # A release built from a red tree is the one kind of release that # cannot be taken back, because the binaries are already on somebody's # disk. This is the same command CI runs. - run: go test -tags "$(cat .github/build-tags)" ./... -count=1 + # + # Including the timeout, and that is the point of saying so. Go allows + # ten minutes PER PACKAGE by default while this job allows thirty for + # all of it, and the suite measured 399 s to 491 s across the three + # systems on 2026-09-03. A release run dying on the default would look + # like a red tree and stop a release that was fine. + run: go test -tags "$(cat .github/build-tags)" ./... -count=1 -timeout 18m - name: what version the code says id: version diff --git a/internal/guard/workflowtimeouts_test.go b/internal/guard/workflowtimeouts_test.go new file mode 100644 index 0000000..e35e361 --- /dev/null +++ b/internal/guard/workflowtimeouts_test.go @@ -0,0 +1,123 @@ +package guard + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/goccy/go-yaml" +) + +// A whole tree test run in a workflow states its own timeout. +// +// Go allows ten minutes PER PACKAGE by default. Nobody chose that, and in this +// repository it is the wrong shape twice over: almost every test lives in one +// package, internal/guard, so the per package limit is effectively the limit +// for the whole suite - and every job here already declares a timeout-minutes +// of its own, which is the number somebody did choose. +// +// When the default fires first, the run does not fail. It PANICS, out of +// whichever test happened to be running when the alarm went off, and the +// output is a goroutine dump rather than a sentence naming anything. This +// project has a written trap for reading that name as the culprit, and it is +// there because the name is innocent. +// +// It has now happened twice. The race detector met it on 2026-08-25 and a long +// comment was written about it - beside that one job. The coverage gate met the +// same thing on 2026-09-03, because a diagnosis recorded at one job does not +// protect the next one. That is what this guard is for: the reasoning is no +// longer kept in a comment next to whichever step learned it. +// +// Measured on 2026-09-03, the run that prompted this: 399 s on ubuntu, 476 s on +// windows, 491 s on macOS, 479 s under coverage instrumentation. Against 600 s. +// And the same branch swung from 479 s to past 600 s between two runs, so the +// margin was smaller than the fleet's own variance. +// +// Only whole tree runs are asked. A targeted -run walks a handful of tests and +// a fuzz step carries -fuzztime, which is its own budget - demanding a flag +// there would be noise, and a guard that cries wolf gets skipped. +func TestEveryWholeTreeTestRunStatesItsOwnTimeout(t *testing.T) { + steps := workflowRunSteps(t) + if len(steps) == 0 { + t.Fatal("no run step was read out of the workflows - this guard would pass against any of them") + } + + asked := 0 + for _, step := range steps { + command := strings.Join(strings.Fields(step.run), " ") + if !strings.Contains(command, "go test ") || !strings.Contains(command, "./...") { + continue + } + asked++ + if strings.Contains(command, "-timeout ") { + continue + } + t.Errorf("%s, step %q runs the whole tree without stating a timeout, so Go's own ten minutes "+ + "a package decides instead of the %s this job declares. A run past it panics out of "+ + "whichever test was running rather than failing at one:\n %s", + step.file, step.name, "timeout-minutes", command) + } + + // The scan has to have found the runs it is judging. Renaming a key, or a + // folded block this stops parsing, would otherwise leave it green while + // reading nothing - the same failure it exists to catch. + if asked < 3 { + t.Errorf("only %d whole tree test runs were found across the workflows, and there are at least "+ + "three - the matrix, the coverage gate and the release. Either they moved or the way this "+ + "reads them stopped working", asked) + } +} + +// runStep is one step that runs a command, with enough around it to say where. +type runStep struct { + file string + name string + run string +} + +// workflowRunSteps reads every run step of every workflow. +// +// Through the YAML parser rather than by searching the text, because a run +// block can be folded over several lines and the flag this guard asks about +// would then sit on a different line from the command. The parser puts them +// back together, which a regular expression over the file would not. +func workflowRunSteps(t *testing.T) []runStep { + t.Helper() + dir := filepath.Join(repoRoot(t), ".github", "workflows") + entries, err := os.ReadDir(dir) + if err != nil { + t.Skipf("the workflows are not here: %v", err) + } + + var out []runStep + for _, e := range entries { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".yml") { + continue + } + body, err := os.ReadFile(filepath.Join(dir, e.Name())) + if err != nil { + t.Fatalf("reading %s: %v", e.Name(), err) + } + var workflow struct { + Jobs map[string]struct { + Steps []struct { + Name string `yaml:"name"` + Run string `yaml:"run"` + } `yaml:"steps"` + } `yaml:"jobs"` + } + if err := yaml.Unmarshal(body, &workflow); err != nil { + t.Fatalf("reading %s: %v", e.Name(), err) + } + for _, job := range workflow.Jobs { + for _, step := range job.Steps { + if step.Run == "" { + continue + } + out = append(out, runStep{file: e.Name(), name: step.Name, run: step.Run}) + } + } + } + return out +}