perf(documents): stop counting every linker's occurrences twice (BUG-2798)

Codex round 13, on what the guard costs the SUCCESS path rather than what it
blocks on the failure path.

The cascade counts occurrences because the size guard needs that number before
it is willing to build anything. It then handed the same content to
links.ReplaceTitle, whose strings.Replace with n < 0 counts it again. Every
ordinary rename therefore paid a second full pass over every linking document
for a number it already had.

links.ReplaceTitleN takes the count the caller already computed. It is a
separate function rather than an optional parameter because the obligation is
real and silent when broken: passing a number that is too small does not
error, it leaves later occurrences unrewritten, which on this path means links
left pointing at a title that no longer exists. A name at the call site is
cheaper than a comment nobody reads.

NO measured speedup is claimed, and the doc comment says so. This removes one
linear pass from a path that also allocates a full copy of the same content and
issues a write per linker, so the saving is real but not obviously
significant. It is here because doing the same work twice needs a reason and
there was not one — not because a benchmark asked for it.

The test asserts equivalence with ReplaceTitle across several shapes, including
the new-title-embeds-old case, and its counterfactual leg asserts that an
under-count visibly DIVERGES — if it did not, the caller's obligation would be
imaginary and the API misleading.

Round 13 also confirmed two things worth recording: no quadratic scan across
linkers, and the ESCAPE clause does not materially change the query plan
because the leading `%` already forced a content scan.

Gates: `go test ./...` under Postgres 17 EXIT=0; gofmt clean; `make lint` 0
issues.

BUG-2798

Claude-Session: https://claude.ai/code/session_011T365kP1N9V88y15HxL4YN
This commit is contained in:
xarmian
2026-08-28 01:52:00 +00:00
parent 65569bf8b5
commit c00606b0c1
3 changed files with 60 additions and 1 deletions
+24
View File
@@ -17,6 +17,30 @@ func ReplaceTitle(content, oldTitle, newTitle string) string {
return replaceAll(content, old, new)
}
// ReplaceTitleN is ReplaceTitle for a caller that has ALREADY counted the
// occurrences, and it exists to stop that work being done twice.
//
// strings.Replace with n < 0 counts the string itself before building the
// result. The document rename cascade counts first anyway — its size guard
// needs the number before it is willing to build anything — so letting
// Replace re-count adds a full pass over every linking document on the
// success path (codex round 13).
//
// n MUST be the count of `[[oldTitle]]` in this exact content. Passing a
// smaller number silently leaves later occurrences unrewritten, which is why
// this is a separate function rather than an optional parameter on the one
// above: the obligation is visible at the call site.
//
// No measured speedup is claimed. This removes one linear pass from a path
// that also allocates a full copy of the same content and issues a write per
// linker, so the saving is real but not obviously significant; it is here
// because doing the same work twice needs a reason and there was not one.
func ReplaceTitleN(content, oldTitle, newTitle string, n int) string {
old := "[[" + oldTitle + "]]"
new := "[[" + newTitle + "]]"
return strings.Replace(content, old, new, n)
}
// RewriteWikiTitle rewrites the four title-form wiki-link shapes that
// resolve to an item titled `oldTitle` in collection `collSlug`,
// substituting `newTitle` for the title portion and preserving any
@@ -71,3 +71,38 @@ func TestReplaceTitle_StillRewritesEveryOccurrence(t *testing.T) {
t.Errorf("an occurrence survived: %q", got)
}
}
// TestReplaceTitleN_MatchesReplaceTitleWhenGivenTheTrueCount pins the
// obligation ReplaceTitleN puts on its caller: given the real occurrence
// count, it must produce exactly what ReplaceTitle produces.
//
// The under-count leg is the counterfactual, and it is why the two are
// separate functions rather than one with an optional parameter — passing a
// number that is too small does not error, it silently leaves later
// occurrences unrewritten, which on the rename path means links left pointing
// at a title that no longer exists.
func TestReplaceTitleN_MatchesReplaceTitleWhenGivenTheTrueCount(t *testing.T) {
for _, tc := range []struct{ name, content, old, new string }{
{"several occurrences", "a [[Old]] b [[Old]] c [[Old]] d", "Old", "New"},
{"none", "nothing to see here", "Old", "New"},
{"new embeds old", "x [[A]] y", "A", "A]] [[A"},
{"shrinking", "[[LongOldTitle]] and [[LongOldTitle]]", "LongOldTitle", "n"},
} {
t.Run(tc.name, func(t *testing.T) {
want := ReplaceTitle(tc.content, tc.old, tc.new)
n := strings.Count(tc.content, "[["+tc.old+"]]")
if got := ReplaceTitleN(tc.content, tc.old, tc.new, n); got != want {
t.Errorf("ReplaceTitleN with the true count %d:\n got: %q\nwant: %q", n, got, want)
}
// Under-counting must visibly diverge, or the count is not
// load-bearing and this function has no contract worth stating.
if n > 1 {
if got := ReplaceTitleN(tc.content, tc.old, tc.new, n-1); got == want {
t.Errorf("ReplaceTitleN with a count one too low produced the correct result; " +
"the caller's obligation is not real, so the API is misleading")
}
}
})
}
}
+1 -1
View File
@@ -687,7 +687,7 @@ func (s *Store) updateLinksInTx(tx *sql.Tx, workspaceID, oldTitle, newTitle stri
return newRenameCascadeTooLargeError(newTitle, retained)
}
du.rewritten = links.ReplaceTitle(du.read, oldTitle, newTitle)
du.rewritten = links.ReplaceTitleN(du.read, oldTitle, newTitle, int(occurrences))
updates = append(updates, du)
}
if err := rows.Err(); err != nil {