From 4c5ccac9983fe5ad7f425489a5edd074701f4b66 Mon Sep 17 00:00:00 2001 From: charles-fineman-bot Date: Mon, 31 Aug 2026 01:40:18 +0000 Subject: [PATCH 1/6] perf: bound the code-span scan, and test the filesystem-error branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #18. A backtick run searches forward for the run that closes it, so a paragraph of runs that never close makes each one rescan everything after it, and the work grows with the cube of the run count. Measured on this branch before the bound: 4.4 s at 4.5 MB, 35 s at 18 MB. One crafted paragraph would spend a CI job's whole budget and surface as a timeout rather than a named failure. The search now gives up after 8 KiB, leaving the run as literal text — which is what an unmatched run already becomes, so no behaviour a real document can express changes. Reaching the ceiling needs a single span longer than 8 KiB; the widest in this repository is under a hundred bytes, and a repository test now fails if one ever approaches it, while the margin is still wide. The timing test took two attempts, and the first was worth recording. At 3000 runs the unbounded scan takes 4.4 s, so a five-second ceiling did not separate the two regimes: with the bound removed the test still passed. At 6000 runs the same input costs 61 ms bounded and 39 s unbounded, so the ceiling now sits eighty times above the passing time and seven times below the failing one. A wall-clock assertion is a poor shape for a test and is used here only because the defect was a growth rate; the size was measured rather than guessed. Closes #15. A small set of filesystem-error branches could not be reached with fstest.MapFS — a directory that fails mid-walk, a root that cannot be listed. A failingFS wrapper reaches them. These are worth testing rather than assuming: had CheckAll treated a collection failure as "no documents found", the entire gate would have passed vacuously on an unreadable tree. Both mutations confirm the tests catch that. Package coverage 94.0% to 96.9%. --- internal/docscheck/docscheck.go | 35 ++++++-- internal/docscheck/docscheck_test.go | 123 ++++++++++++++++++++++++++ internal/docscheck/repository_test.go | 36 ++++++++ 3 files changed, 188 insertions(+), 6 deletions(-) diff --git a/internal/docscheck/docscheck.go b/internal/docscheck/docscheck.go index 9fa1f2a..4390628 100644 --- a/internal/docscheck/docscheck.go +++ b/internal/docscheck/docscheck.go @@ -32,12 +32,14 @@ // live Markdown. A repository path or a link quoted inside such a // span is therefore never checked. No multi-line span appears in // this repository's documentation today. -// - A backtick run that never closes is scanned to the end of its -// paragraph, so a paragraph built entirely of such runs costs more -// than linear time. Checking this repository's real tree takes a few -// tens of milliseconds, and the growth starts to matter only for a -// crafted document orders of magnitude larger than the tree itself. -// Bounding the scan is tracked separately. +// - A backtick run's search for its closing run gives up after +// maxCodeSpanScanBytes, leaving the run as literal text. That is what +// an unmatched run already becomes, so the ceiling changes no +// behavior a real document can observe: it takes a span longer than +// the bound to reach one, and the longest in this repository is two +// orders of magnitude below it. Without the ceiling, a paragraph +// built entirely of never-closing runs costs superlinear time, +// because each run rescans the rest of the paragraph. // - Indented (non-fenced) code blocks are not treated as examples. Every // example here uses a fence. // - The checks trust the working tree's file identity, not only its path @@ -75,6 +77,21 @@ const ( releaseWorkflowFile = ".github/workflows/release.yml" ) +// maxCodeSpanScanBytes bounds how far one backtick run may search for the +// run that closes it. Beyond the bound the run is left as literal text — +// the same outcome an unmatched run already produces, so nothing a real +// document can express changes: reaching the ceiling needs a single code +// span longer than 8 KiB, where the longest in this repository is under a +// hundred bytes. +// +// The ceiling exists because the search is per run: a paragraph of runs +// that never close makes each one rescan everything after it. Measured +// before the bound, a paragraph of runs of increasing length cost 163 ms +// at 20 KB, 1.8 s at 81 KB, and 26.7 s at 321 KB — a shape that would +// spend a CI job's whole budget inside one paragraph and surface as a +// timeout rather than as a named failure. +const maxCodeSpanScanBytes = 8 << 10 + // selfCheckStep is the workflow step whose command list AGENTS.md declares // authoritative. const selfCheckStep = "Self-check fixture documents" @@ -799,6 +816,7 @@ func codeSpanEnd(lines []string, index, position int) (endIndex, endPosition int } opening := leadingRun(lines[index][position:], '`') scan := position + opening + budget := maxCodeSpanScanBytes for cursor := index; cursor < len(lines); cursor++ { if cursor > index { if strings.TrimSpace(lines[cursor]) == "" { @@ -808,8 +826,12 @@ func codeSpanEnd(lines []string, index, position int) (endIndex, endPosition int } line := lines[cursor] for scan < len(line) { + if budget <= 0 { + return 0, 0, false + } if line[scan] != '`' { scan++ + budget-- continue } run := leadingRun(line[scan:], '`') @@ -817,6 +839,7 @@ func codeSpanEnd(lines []string, index, position int) (endIndex, endPosition int return cursor, scan + opening, true } scan += run + budget -= run } } return 0, 0, false diff --git a/internal/docscheck/docscheck_test.go b/internal/docscheck/docscheck_test.go index dc9cfbe..af045f3 100644 --- a/internal/docscheck/docscheck_test.go +++ b/internal/docscheck/docscheck_test.go @@ -1,10 +1,13 @@ package docscheck import ( + "errors" + "io/fs" "maps" "strings" "testing" "testing/fstest" + "time" ) // The fixture repository is a miniature of this one: the same file @@ -849,3 +852,123 @@ func TestDocumentFilesSkipsFixturesAndToolDirectories(t *testing.T) { t.Fatal("document set is empty") } } + +// TestCodeSpanScanIsBounded is the regression test for the cost curve that +// motivated maxCodeSpanScanBytes. A paragraph of backtick runs that never +// close makes each run search everything after it, so the work grows with +// the cube of the run count. +// +// A wall-clock assertion is ordinarily a poor shape for a test, and is +// used here because the defect was a growth rate rather than a wrong +// answer. The size was chosen by measuring both regimes rather than +// guessing: at 3000 runs the unbounded scan takes 4.4 s, which a five +// second ceiling does not reliably separate from anything — an earlier +// version of this test passed with the bound removed. At 6000 runs the +// same input costs 61 ms bounded and 35 s unbounded, so the ceiling below +// sits eighty times above the passing time and seven times beneath the +// failing one. +// +// Scanning starts at each run rather than at each backtick, which is how +// readCodeSpan actually calls it. +func TestCodeSpanScanIsBounded(t *testing.T) { + var builder strings.Builder + for run := 1; run <= 6000; run++ { + builder.WriteString(strings.Repeat("`", run)) + builder.WriteString("x") + } + line := builder.String() + lines := []string{line} + + start := time.Now() + for index := 0; index < len(line); { + if line[index] != '`' { + index++ + continue + } + codeSpanEnd(lines, 0, index) + index += leadingRun(line[index:], '`') + } + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("scanning %d bytes of unclosed runs took %v", len(line), elapsed) + } +} + +// TestCodeSpanBoundLeavesRealSpansIntact is the other half: the ceiling +// must be unreachable by anything a document would legitimately contain. +// A span just under the bound still closes; one past it is left as literal +// text, which is exactly what an unmatched run already becomes. +func TestCodeSpanBoundLeavesRealSpansIntact(t *testing.T) { + t.Run("span just inside the bound closes", func(t *testing.T) { + line := "`" + strings.Repeat("x", maxCodeSpanScanBytes-16) + "`" + if _, _, ok := codeSpanEnd([]string{line}, 0, 0); !ok { + t.Fatal("a span within the bound must still close") + } + }) + + t.Run("span past the bound is literal text", func(t *testing.T) { + line := "`" + strings.Repeat("x", maxCodeSpanScanBytes+16) + "`" + if _, _, ok := codeSpanEnd([]string{line}, 0, 0); ok { + t.Fatal("a span past the bound must be left unclosed") + } + }) + +} + +// failingFS wraps an fs.FS and returns a chosen error for the operations +// that fstest.MapFS cannot be made to fail. The error branches it reaches +// are the ones a real tree hits — a directory that becomes unreadable +// mid-walk, a root that cannot be listed — so leaving them untested meant +// trusting that a failure there is reported rather than silently treated +// as "no documents found", which would make the whole gate pass vacuously. +type failingFS struct { + fs.FS + // failOpen names the path whose Open fails; empty means none. + failOpen string + // failReadDir names the directory whose listing fails; empty means none. + failReadDir string + err error +} + +func (f failingFS) Open(name string) (fs.File, error) { + if f.failOpen != "" && name == f.failOpen { + return nil, f.err + } + return f.FS.Open(name) +} + +func (f failingFS) ReadDir(name string) ([]fs.DirEntry, error) { + if f.failReadDir != "" && name == f.failReadDir { + return nil, f.err + } + return fs.ReadDir(f.FS, name) +} + +func TestFilesystemErrorsAreReported(t *testing.T) { + broken := errors.New("disk went away") + + t.Run("walk failure surfaces from DocumentFiles", func(t *testing.T) { + fsys := failingFS{FS: repository(nil), failReadDir: "docs", err: broken} + if _, err := DocumentFiles(fsys); !errors.Is(err, broken) { + t.Fatalf("expected the walk error, got %v", err) + } + }) + + t.Run("CheckAll reports a collection failure rather than passing", func(t *testing.T) { + fsys := failingFS{FS: repository(nil), failReadDir: ".", err: broken} + errs := CheckAll(fsys) + if len(errs) == 0 { + t.Fatal("a tree that cannot be walked must not pass the gate") + } + if !strings.Contains(errs.Error(), "collect Markdown documents") { + t.Fatalf("expected a collection failure, got:\n%s", errs) + } + }) + + t.Run("root listing failure surfaces from CheckNamedPaths", func(t *testing.T) { + fsys := failingFS{FS: repository(nil), failReadDir: ".", err: broken} + errs := CheckNamedPaths(fsys, []string{agentsFile}) + if len(errs) == 0 { + t.Fatal("an unreadable root must not silently check nothing") + } + }) +} diff --git a/internal/docscheck/repository_test.go b/internal/docscheck/repository_test.go index 7ea3a0a..8fa57cc 100644 --- a/internal/docscheck/repository_test.go +++ b/internal/docscheck/repository_test.go @@ -1,7 +1,9 @@ package docscheck import ( + "io/fs" "os" + "strings" "testing" "github.com/sofired/tracedoc/internal/testsupport" @@ -57,3 +59,37 @@ func TestRepositoryDocumentFiles(t *testing.T) { } } } + +// TestRepositoryCodeSpansStayInsideScanBound guards the claim +// maxCodeSpanScanBytes rests on: that no document here carries a code span +// anywhere near the ceiling, so the bound cannot change how a real document +// is read. If one ever approaches it, this fails while the margin is still +// wide rather than after a span silently stops closing. +func TestRepositoryCodeSpansStayInsideScanBound(t *testing.T) { + const widest = maxCodeSpanScanBytes / 8 + + root := os.DirFS(testsupport.Path(t)) + files, err := DocumentFiles(root) + if err != nil { + t.Fatalf("collect documents: %v", err) + } + for _, name := range files { + data, err := fs.ReadFile(root, name) + if err != nil { + t.Fatalf("read %s: %v", name, err) + } + for number, line := range strings.Split(string(data), "\n") { + for index := 0; index < len(line); index++ { + if line[index] != '`' { + continue + } + if width := codeSpanAt(line, index); width > widest { + t.Errorf( + "%s:%d has a %d-byte code span, approaching the %d-byte scan bound", + name, number+1, width, maxCodeSpanScanBytes, + ) + } + } + } + } +} From 5defd35af101178fbc9ff41f1aded937c0f81d9d Mon Sep 17 00:00:00 2001 From: charles-fineman-bot Date: Mon, 31 Aug 2026 01:47:17 +0000 Subject: [PATCH 2/6] revert: drop the filesystem-error tests, which PR #20 already covers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #20 was opened for #15 five minutes before this branch's pull request and implements the same thing — an fs.FS that fails on ReadDir, exercising the walk and collection error paths. Two sessions picked up the issue in parallel. Theirs was first and is scoped to #15 alone, where this branch only carried those tests because #18 touches the same package. Removing them here resolves the conflict in docscheck_test.go and leaves this branch as what it should have been: the scan bound and nothing else. The #18 work is untouched. Package coverage returns to 95.5%, with #20 carrying the rest. --- internal/docscheck/docscheck_test.go | 61 ---------------------------- 1 file changed, 61 deletions(-) diff --git a/internal/docscheck/docscheck_test.go b/internal/docscheck/docscheck_test.go index af045f3..69e1d32 100644 --- a/internal/docscheck/docscheck_test.go +++ b/internal/docscheck/docscheck_test.go @@ -1,8 +1,6 @@ package docscheck import ( - "errors" - "io/fs" "maps" "strings" "testing" @@ -913,62 +911,3 @@ func TestCodeSpanBoundLeavesRealSpansIntact(t *testing.T) { }) } - -// failingFS wraps an fs.FS and returns a chosen error for the operations -// that fstest.MapFS cannot be made to fail. The error branches it reaches -// are the ones a real tree hits — a directory that becomes unreadable -// mid-walk, a root that cannot be listed — so leaving them untested meant -// trusting that a failure there is reported rather than silently treated -// as "no documents found", which would make the whole gate pass vacuously. -type failingFS struct { - fs.FS - // failOpen names the path whose Open fails; empty means none. - failOpen string - // failReadDir names the directory whose listing fails; empty means none. - failReadDir string - err error -} - -func (f failingFS) Open(name string) (fs.File, error) { - if f.failOpen != "" && name == f.failOpen { - return nil, f.err - } - return f.FS.Open(name) -} - -func (f failingFS) ReadDir(name string) ([]fs.DirEntry, error) { - if f.failReadDir != "" && name == f.failReadDir { - return nil, f.err - } - return fs.ReadDir(f.FS, name) -} - -func TestFilesystemErrorsAreReported(t *testing.T) { - broken := errors.New("disk went away") - - t.Run("walk failure surfaces from DocumentFiles", func(t *testing.T) { - fsys := failingFS{FS: repository(nil), failReadDir: "docs", err: broken} - if _, err := DocumentFiles(fsys); !errors.Is(err, broken) { - t.Fatalf("expected the walk error, got %v", err) - } - }) - - t.Run("CheckAll reports a collection failure rather than passing", func(t *testing.T) { - fsys := failingFS{FS: repository(nil), failReadDir: ".", err: broken} - errs := CheckAll(fsys) - if len(errs) == 0 { - t.Fatal("a tree that cannot be walked must not pass the gate") - } - if !strings.Contains(errs.Error(), "collect Markdown documents") { - t.Fatalf("expected a collection failure, got:\n%s", errs) - } - }) - - t.Run("root listing failure surfaces from CheckNamedPaths", func(t *testing.T) { - fsys := failingFS{FS: repository(nil), failReadDir: ".", err: broken} - errs := CheckNamedPaths(fsys, []string{agentsFile}) - if len(errs) == 0 { - t.Fatal("an unreadable root must not silently check nothing") - } - }) -} From fe1e9df61352a0c81bc8237496b6585ee92851e5 Mon Sep 17 00:00:00 2001 From: charles-fineman-bot Date: Mon, 31 Aug 2026 03:08:05 +0000 Subject: [PATCH 3/6] perf: bound the candidate run a code-span scan measures maxCodeSpanScanBytes was spent per byte stepped over, but the candidate run a scan landed on was measured whole before that accounting ran. One oversized run therefore cost its full length no matter how little budget remained, and because runs of strictly increasing length never match each other, every run in a prefix that fits inside the budget reaches that run and repeats the read. Measure a candidate only far enough to tell an exact match from a longer run, and never past the remaining budget. A run that reaches the limit is a non-match, which is what the budget running out already produces. An input of 125 increasing runs before a 64 MiB run cost 4.5 s before and 79 ms after. --- internal/docscheck/docscheck.go | 23 +++++++-- internal/docscheck/docscheck_test.go | 74 ++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/internal/docscheck/docscheck.go b/internal/docscheck/docscheck.go index 4390628..41c140f 100644 --- a/internal/docscheck/docscheck.go +++ b/internal/docscheck/docscheck.go @@ -834,8 +834,17 @@ func codeSpanEnd(lines []string, index, position int) (endIndex, endPosition int budget-- continue } - run := leadingRun(line[scan:], '`') - if run == opening { + // Measuring only far enough to tell an exact match from a + // longer run is what keeps one oversized run inside the + // budget: reading the run whole would spend its full length + // before the check above ever saw the cost, and the runs + // that precede it each repeat that read. + limit := opening + 1 + if limit > budget { + limit = budget + } + run := leadingRunAtMost(line[scan:], '`', limit) + if run == opening && run < limit { return cursor, scan + opening, true } scan += run @@ -847,8 +856,16 @@ func codeSpanEnd(lines []string, index, position int) (endIndex, endPosition int // leadingRun counts the leading repetitions of char in value. func leadingRun(value string, char byte) int { + return leadingRunAtMost(value, char, len(value)) +} + +// leadingRunAtMost counts the leading repetitions of char in value, +// stopping once limit of them have been seen. A caller that only needs to +// tell one exact length from every greater one then pays for the length +// it asked about rather than for the run it was handed. +func leadingRunAtMost(value string, char byte, limit int) int { run := 0 - for run < len(value) && value[run] == char { + for run < limit && run < len(value) && value[run] == char { run++ } return run diff --git a/internal/docscheck/docscheck_test.go b/internal/docscheck/docscheck_test.go index 69e1d32..7c3ed3f 100644 --- a/internal/docscheck/docscheck_test.go +++ b/internal/docscheck/docscheck_test.go @@ -891,6 +891,80 @@ func TestCodeSpanScanIsBounded(t *testing.T) { } } +// TestCodeSpanScanBoundsOversizedCandidateRun covers the way a single run +// used to escape maxCodeSpanScanBytes. The budget was spent per byte +// stepped over, but the candidate run a scan landed on was measured whole +// before that accounting ran, so one enormous run cost its full length no +// matter how little budget was left. +// +// Runs of strictly increasing length never match each other, so every run +// in the prefix below travels the whole way to the oversized run and pays +// for it again. The prefix is sized to sit just under the budget, which +// is what keeps all of it in play: 125 runs reach the 64 MiB run, and the +// unbounded measurement reads it 125 times. +// +// The wall-clock assertion carries the same caveat as +// TestCodeSpanScanIsBounded, and the same measurement behind it. Bounded, +// this input takes 79 ms — one unavoidable pass, made when splitCodeSpans +// reaches the oversized run and measures it as an opening run of its own. +// Unbounded it takes 4.5 s, so the ceiling below sits twelve times above +// the passing time and four times beneath the failing one. The race +// detector does not move the passing time, which is a single tight loop +// over bytes nothing else touches. +func TestCodeSpanScanBoundsOversizedCandidateRun(t *testing.T) { + var builder strings.Builder + for run := 1; run <= 125; run++ { + builder.WriteString(strings.Repeat("`", run)) + builder.WriteString("x") + } + if builder.Len() >= maxCodeSpanScanBytes { + t.Fatalf("prefix of %d bytes must stay under the %d byte budget", + builder.Len(), maxCodeSpanScanBytes) + } + builder.WriteString(strings.Repeat("`", 1<<26)) + line := builder.String() + lines := []string{line} + + start := time.Now() + for index := 0; index < len(line); { + if line[index] != '`' { + index++ + continue + } + codeSpanEnd(lines, 0, index) + index += leadingRun(line[index:], '`') + } + if elapsed := time.Since(start); elapsed > time.Second { + t.Fatalf("scanning %d bytes ending in an oversized run took %v", len(line), elapsed) + } +} + +// TestCodeSpanRunLengthsMustMatchExactly pins the distinction the bounded +// measurement has to preserve. Measuring a candidate run only far enough +// to tell it from an exact match means the closing rule is now enforced by +// a length limit rather than by a full count, so a run one backtick too +// long has to stay a non-match. +func TestCodeSpanRunLengthsMustMatchExactly(t *testing.T) { + cases := []struct { + name string + line string + close bool + }{ + {"equal runs close", "``x``", true}, + {"longer candidate does not close", "``x```", false}, + {"shorter candidate does not close", "``x`", false}, + {"longer candidate before an equal one", "`x``x`", true}, + } + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + _, _, ok := codeSpanEnd([]string{test.line}, 0, 0) + if ok != test.close { + t.Fatalf("codeSpanEnd(%q) closed = %v, want %v", test.line, ok, test.close) + } + }) + } +} + // TestCodeSpanBoundLeavesRealSpansIntact is the other half: the ceiling // must be unreachable by anything a document would legitimately contain. // A span just under the bound still closes; one past it is left as literal From 3b6fe145d6762b01184726d5aec0e5efba80cfde Mon Sep 17 00:00:00 2001 From: charles-fineman-bot Date: Mon, 31 Aug 2026 03:18:12 +0000 Subject: [PATCH 4/6] fix: keep the bounded run measurement faithful at both edges Peer review found the bounded measurement wrong in two opposite directions, both introduced by fe1e9df. A measurement that stops at the limit stops inside the run, and the search resumed at that offset, reading the remainder as a run of its own. A run of five backticks therefore closed a two-backtick opening on its last two, pulling the prose between them into a code span and out of the link and heading checks. Step over the remainder instead, under the same budget. Clamping the limit to the remaining budget also left a run of exactly opening backticks indistinguishable from a longer one, and calling that unmatched turned a closing run landing on the last of the budget into literal text. Read one past the opening length regardless: that costs the opening run's own length once per call, against a budget the next iteration is about to find spent. Both were verified against the unbounded algorithm this replaced, exhaustively over every backtick arrangement up to fourteen bytes and every two-line pair up to seven, plus a sweep of the budget boundary: zero disagreements, where the previous commit disagreed on 7432 of the two-line cases. The oversized-run bound still holds at 79 ms. --- internal/docscheck/docscheck.go | 37 +++++++++++++++++------- internal/docscheck/docscheck_test.go | 43 ++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 12 deletions(-) diff --git a/internal/docscheck/docscheck.go b/internal/docscheck/docscheck.go index 41c140f..cf0d46e 100644 --- a/internal/docscheck/docscheck.go +++ b/internal/docscheck/docscheck.go @@ -837,18 +837,33 @@ func codeSpanEnd(lines []string, index, position int) (endIndex, endPosition int // Measuring only far enough to tell an exact match from a // longer run is what keeps one oversized run inside the // budget: reading the run whole would spend its full length - // before the check above ever saw the cost, and the runs - // that precede it each repeat that read. - limit := opening + 1 - if limit > budget { - limit = budget - } - run := leadingRunAtMost(line[scan:], '`', limit) - if run == opening && run < limit { + // before the check above ever saw the cost, and every run + // that precedes it repeats that read. + // + // The limit is not clamped to the remaining budget. Doing so + // would leave a run of exactly opening backticks + // indistinguishable from a longer one, and the cheap reading + // -- calling it unmatched -- turns a closing run that lands + // on the last of the budget into literal text. Reading one + // past opening costs at most the opening run's own length, + // once per call, against a budget the next iteration is + // about to find spent. + run := leadingRunAtMost(line[scan:], '`', opening+1) + if run == opening { return cursor, scan + opening, true } scan += run budget -= run + // A measurement that stopped at the limit may have stopped + // inside the run rather than at its end, so the remainder is + // stepped over here. Resuming the search at that offset + // instead would read what is left as a run of its own and + // could take its tail for the closing one: ``x````` would + // close on the last two backticks of a run of five. + for budget > 0 && scan < len(line) && line[scan] == '`' { + scan++ + budget-- + } } } return 0, 0, false @@ -860,9 +875,11 @@ func leadingRun(value string, char byte) int { } // leadingRunAtMost counts the leading repetitions of char in value, -// stopping once limit of them have been seen. A caller that only needs to +// stopping once it has counted limit of them. A caller that only needs to // tell one exact length from every greater one then pays for the length -// it asked about rather than for the run it was handed. +// it asked about rather than for the run it was handed, but it gets back +// a count that may sit inside a run rather than at its end, and has to +// step over the remainder itself. func leadingRunAtMost(value string, char byte, limit int) int { run := 0 for run < limit && run < len(value) && value[run] == char { diff --git a/internal/docscheck/docscheck_test.go b/internal/docscheck/docscheck_test.go index 7c3ed3f..2bcee6c 100644 --- a/internal/docscheck/docscheck_test.go +++ b/internal/docscheck/docscheck_test.go @@ -941,9 +941,15 @@ func TestCodeSpanScanBoundsOversizedCandidateRun(t *testing.T) { // TestCodeSpanRunLengthsMustMatchExactly pins the distinction the bounded // measurement has to preserve. Measuring a candidate run only far enough -// to tell it from an exact match means the closing rule is now enforced by -// a length limit rather than by a full count, so a run one backtick too +// to tell it from an exact match means the closing rule is enforced by a +// length limit rather than by a full count, so a run one backtick too // long has to stay a non-match. +// +// The last three cases guard the trap that shape sets. A measurement that +// stops at the limit stops inside the run, and a search that resumed +// there would read the remainder as a run of its own: a run of five +// backticks would close a two-backtick opening on its last two, silently +// pulling whatever lay between into a code span and out of the checks. func TestCodeSpanRunLengthsMustMatchExactly(t *testing.T) { cases := []struct { name string @@ -954,6 +960,9 @@ func TestCodeSpanRunLengthsMustMatchExactly(t *testing.T) { {"longer candidate does not close", "``x```", false}, {"shorter candidate does not close", "``x`", false}, {"longer candidate before an equal one", "`x``x`", true}, + {"run far longer than the opening", "``x`````", false}, + {"run three times the opening", "`x```", false}, + {"remainder of a long run is not a new run", "```x```````", false}, } for _, test := range cases { t.Run(test.name, func(t *testing.T) { @@ -965,6 +974,36 @@ func TestCodeSpanRunLengthsMustMatchExactly(t *testing.T) { } } +// TestCodeSpanClosesOnTheLastOfTheBudget pins the other edge of the +// bounded measurement. A closing run that lands exactly where the budget +// runs out is still a closing run, so the measurement reads one backtick +// past the opening length even when the budget cannot cover it: a +// candidate capped at the budget would be indistinguishable from a longer +// run, and calling that unmatched would turn a legitimate span into +// literal text a few bytes short of the documented ceiling. +// +// The second case is why the first cannot simply be waved through. At the +// same offset, a run longer than the opening must still fail to close. +func TestCodeSpanClosesOnTheLastOfTheBudget(t *testing.T) { + const opening = 3 + filler := strings.Repeat("x", maxCodeSpanScanBytes-opening) + run := strings.Repeat("`", opening) + + t.Run("an exact run on the last of the budget closes", func(t *testing.T) { + line := run + filler + run + if _, _, ok := codeSpanEnd([]string{line}, 0, 0); !ok { + t.Fatal("a closing run reached within the budget must still close") + } + }) + + t.Run("a longer run on the last of the budget does not", func(t *testing.T) { + line := run + filler + run + "``" + if _, _, ok := codeSpanEnd([]string{line}, 0, 0); ok { + t.Fatal("a run longer than the opening must never close it") + } + }) +} + // TestCodeSpanBoundLeavesRealSpansIntact is the other half: the ceiling // must be unreachable by anything a document would legitimately contain. // A span just under the bound still closes; one past it is left as literal From 026e8490353a345431e520e214ea9379995c659b Mon Sep 17 00:00:00 2001 From: charles-fineman-bot Date: Mon, 31 Aug 2026 03:24:02 +0000 Subject: [PATCH 5/6] fix: hold the opening run to the scan ceiling as well The ceiling governed how far a run searched, but not how long a run could be, and the two delimiters of a span are runs. Two runs of 8 KiB with a byte between them therefore closed a span after inspecting 16 KiB, which is the span the ceiling is meant to have given up on. The cost stays linear either way, so this is the documented bound drifting from the code rather than the timeout returning, but the bound is worth keeping honest: the measurement below reads one past the opening length, so an unbounded opening run also set an unbounded floor under every candidate it compared against. Measure the opening run under the same ceiling and leave one past it as literal text. One call now reads at most twice the ceiling -- an opening run at it, then a search running the same distance -- and the const comment says so rather than presenting the ceiling as exact. Also adds the multi-line cases peer review asked for. codeSpanEnd exists for spans that close on a later line, and nothing committed covered that path; the run lengths have to match across a line break exactly as they do within one, and a blank line still ends the paragraph first. --- internal/docscheck/docscheck.go | 48 ++++++++++++++++++---------- internal/docscheck/docscheck_test.go | 48 ++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 16 deletions(-) diff --git a/internal/docscheck/docscheck.go b/internal/docscheck/docscheck.go index cf0d46e..03e701e 100644 --- a/internal/docscheck/docscheck.go +++ b/internal/docscheck/docscheck.go @@ -32,9 +32,9 @@ // live Markdown. A repository path or a link quoted inside such a // span is therefore never checked. No multi-line span appears in // this repository's documentation today. -// - A backtick run's search for its closing run gives up after -// maxCodeSpanScanBytes, leaving the run as literal text. That is what -// an unmatched run already becomes, so the ceiling changes no +// - A backtick run longer than maxCodeSpanScanBytes, or whose search +// for its closing run runs that far, is left as literal text. That is +// what an unmatched run already becomes, so the ceiling changes no // behavior a real document can observe: it takes a span longer than // the bound to reach one, and the longest in this repository is two // orders of magnitude below it. Without the ceiling, a paragraph @@ -77,12 +77,19 @@ const ( releaseWorkflowFile = ".github/workflows/release.yml" ) -// maxCodeSpanScanBytes bounds how far one backtick run may search for the -// run that closes it. Beyond the bound the run is left as literal text — -// the same outcome an unmatched run already produces, so nothing a real -// document can express changes: reaching the ceiling needs a single code -// span longer than 8 KiB, where the longest in this repository is under a -// hundred bytes. +// maxCodeSpanScanBytes bounds both how long a backtick run may be and how +// far it may search for the run that closes it. Beyond the bound the run +// is left as literal text — the same outcome an unmatched run already +// produces, so nothing a real document can express changes: reaching the +// ceiling needs a single code span longer than 8 KiB, where the longest +// in this repository is under a hundred bytes. +// +// The bound is a ceiling on each of the two, not on their sum, so one +// call reads at most twice it: an opening run at the ceiling, then a +// search that runs the same distance. Measuring a candidate run also +// reads one byte past the opening's length, which is what tells an exact +// closing run from a longer one, and that byte can fall past a budget +// the next iteration is about to find spent. // // The ceiling exists because the search is per run: a paragraph of runs // that never close makes each one rescan everything after it. Measured @@ -814,7 +821,16 @@ func codeSpanEnd(lines []string, index, position int) (endIndex, endPosition int if position >= len(lines[index]) || lines[index][position] != '`' { return 0, 0, false } - opening := leadingRun(lines[index][position:], '`') + // The opening run is measured under the same ceiling as the search + // that follows it, and a run past the ceiling is left as literal + // text. Without that, a pair of oversized delimiters would close a + // span the ceiling is meant to have given up on -- two runs of 8 KiB + // and one byte between them inspect 16 KiB -- and every measurement + // below would be free to read a run that long. + opening := leadingRunAtMost(lines[index][position:], '`', maxCodeSpanScanBytes+1) + if opening > maxCodeSpanScanBytes { + return 0, 0, false + } scan := position + opening budget := maxCodeSpanScanBytes for cursor := index; cursor < len(lines); cursor++ { @@ -842,12 +858,12 @@ func codeSpanEnd(lines []string, index, position int) (endIndex, endPosition int // // The limit is not clamped to the remaining budget. Doing so // would leave a run of exactly opening backticks - // indistinguishable from a longer one, and the cheap reading - // -- calling it unmatched -- turns a closing run that lands - // on the last of the budget into literal text. Reading one - // past opening costs at most the opening run's own length, - // once per call, against a budget the next iteration is - // about to find spent. + // indistinguishable from a longer one, and resolving that + // ambiguity as a non-match turns a closing run landing on + // the last of the budget into literal text. Reading one past + // opening costs at most the opening run's own length, once + // per call, against a budget the next iteration is about to + // find spent. run := leadingRunAtMost(line[scan:], '`', opening+1) if run == opening { return cursor, scan + opening, true diff --git a/internal/docscheck/docscheck_test.go b/internal/docscheck/docscheck_test.go index 2bcee6c..4fe4f8a 100644 --- a/internal/docscheck/docscheck_test.go +++ b/internal/docscheck/docscheck_test.go @@ -1004,6 +1004,54 @@ func TestCodeSpanClosesOnTheLastOfTheBudget(t *testing.T) { }) } +// TestCodeSpanDelimitersObeyTheBound pins the ceiling against the runs +// themselves rather than against the distance between them. Two runs of +// equal length with a byte between them close a span whatever their +// length, so measuring the opening run without a ceiling would let a pair +// of 8 KiB delimiters inspect 16 KiB -- a span the bound is meant to have +// given up on. The opening run is measured under the ceiling too, and one +// past it is literal text. +func TestCodeSpanDelimitersObeyTheBound(t *testing.T) { + closes := func(length int) bool { + run := strings.Repeat("`", length) + _, _, ok := codeSpanEnd([]string{run + "x" + run}, 0, 0) + return ok + } + if !closes(maxCodeSpanScanBytes) { + t.Error("delimiters at the ceiling must still close") + } + if closes(maxCodeSpanScanBytes + 1) { + t.Error("delimiters past the ceiling must be literal text") + } +} + +// TestCodeSpanClosesOnALaterLine covers the multi-line path through the +// same measurement. A span whose closing run sits on a later line is the +// case codeSpanEnd exists for -- a quoted "