From 2c40146c00f24287647695a248c220ad726c094c Mon Sep 17 00:00:00 2001 From: jordanrburger Date: Thu, 3 Sep 2026 13:21:32 -0400 Subject: [PATCH 1/2] perf(action-items): de-quadratic the parse, guard the linkifier, LRU the md cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured first paint on a real 1.8 MB day (683 live + 124 archived tasks, Debug): 2.9 s total — 11 ms file read, 830 ms parse, ~2.0 s SwiftUI. This takes ~535 ms off the non-SwiftUI half. The SwiftUI 71% is untouched and is the next piece of work. 1. `splitSubjectBody` was quadratic — 830 ms parse → 369 ms. `firstSeparatorOutsideTokens` walked `String.Index` and called `text.distance(from:to:)` — itself O(n) — three times per character, so cost grew with the square of the line length. Task lines run to 9,685 chars. Synthetic scaling confirmed it: doubling the input quadrupled the time (8k chars 100 ms, 16k 410 ms), and it was optimization-immune because the cost is stdlib grapheme-breaking, not unoptimized Swift. Rewritten over a `[Character]` array with integer indices. The function itself goes 562 ms → 122 ms over all 807 real task lines. Behavior must not move: `parser-corpus.json` is byte-identical across three repos but carries only 6 entries with a body, so the safety net is a differential test holding the original implementation verbatim and agreeing with it across ~2,900 generated inputs covering every scanner branch. 2. `GitHubRefLinkifier.linkify` had no fast path — 129 ms → 74 ms. It ran three protected-range regexes plus two repo-inference regexes over every string before checking whether a ref could exist. `KBTag.linkify` already had the guard; this didn't. Guarding on a bare `#` is not enough here, and measuring said so: Scout vaults are dense with `[#TAG]` mnemonics and `#channel` names, so 1,089 of 2,307 rendered strings contain a `#`. Both branches of `refRe` need a `#` immediately followed by a digit, so the guard tests for that instead — which also correctly admits digit-leading tags like `[#5864M]`, since `#5` clears it (they're then left alone by the regex, as before). 3. The markdown cache evicted arbitrarily — warm re-render 320 ms → 24 ms. At the 2000 cap it dropped `cache.keys.prefix(1000)`, i.e. arbitrary dictionary order, so the strings currently on screen were as likely to go as any other. Past the cap that became a re-parse treadmill: a warm pass over a 2,398-string working set measured ~320 ms against 12 ms uncapped, a 27x cliff hit by expanding cards, searching and switching days. Now a true LRU keyed on a logical clock, evicting the coldest quarter, so crossing the cap degrades gradually instead of falling off an edge. Cap raised 2000 → 6000 (first paint alone touches ~935 uniques). Co-Authored-By: Claude Opus 5 --- Scout/ActionItems/ActionItemsParser.swift | 113 +++++--- .../Views/GitHubRefLinkifier.swift | 27 ++ .../Views/InlineMarkdownText.swift | 67 ++++- .../ActionItems/GitHubRefLinkifierTests.swift | 73 +++++ .../InlineMarkdownCacheTests.swift | 84 ++++++ .../ActionItems/SplitSubjectBodyTests.swift | 272 ++++++++++++++++++ 6 files changed, 580 insertions(+), 56 deletions(-) create mode 100644 ScoutTests/ActionItems/InlineMarkdownCacheTests.swift create mode 100644 ScoutTests/ActionItems/SplitSubjectBodyTests.swift diff --git a/Scout/ActionItems/ActionItemsParser.swift b/Scout/ActionItems/ActionItemsParser.swift index 1ed4a8a..e98b057 100644 --- a/Scout/ActionItems/ActionItemsParser.swift +++ b/Scout/ActionItems/ActionItemsParser.swift @@ -778,65 +778,90 @@ extension ActionItemsParser { /// and ``[label](url)`` tokens. Falls back to ``": "`` separator. Mirrors /// ``action-items/render.py`` ``_split_subject``. static func splitSubjectBody(_ rest: String) -> (String, String) { - let separators = [" — ", " – ", " - "] - if let idx = firstSeparatorOutsideTokens(in: rest, separators: separators) { - for sep in separators { - let sepLen = sep.count - if rest.distance(from: idx, to: rest.endIndex)>= sepLen, - rest[idx ..< rest.index(idx, offsetBy: sepLen)] == sep { - return ( - String(rest[.. String.Index? { + /// ` — ` / ` – ` / ` - `, in the order the scanner tries them at each + /// position. Hoisted so the arrays aren't rebuilt per task line. + private static let dashSeparators: [[Character]] = [ + Array(" — "), Array(" – "), Array(" - "), + ] + private static let colonSeparator: [[Character]] = [Array(": ")] + + private static func split( + _ chars: [Character], + at hit: (index: Int, length: Int) + ) -> (String, String) { + ( + String(chars[.. (index: Int, length: Int)? { var inBold = false, inStrike = false, inCode = false var bracketDepth = 0, parenDepth = 0 - var i = text.startIndex - while i < text.endIndex { - let rem = text[i...] - let two = rem.prefix(2) - let ch = text[i] - if ch == "`" && !inBold && !inStrike { inCode.toggle(); i = text.index(after: i); continue } - if inCode { i = text.index(after: i); continue } - if two == "**" { inBold.toggle(); i = text.index(i, offsetBy: 2); continue } - if two == "~~" { inStrike.toggle(); i = text.index(i, offsetBy: 2); continue } - if two == "[[" { bracketDepth += 1; i = text.index(i, offsetBy: 2); continue } - if two == "]]" && bracketDepth> 0 { bracketDepth -= 1; i = text.index(i, offsetBy: 2); continue } - if ch == "[" && bracketDepth == 0 { bracketDepth = 1; i = text.index(after: i); continue } - if ch == "]" && bracketDepth> 0 && two != "]]" { + var i = 0 + let n = chars.count + while i < n { + let ch = chars[i] + let next: Character? = i + 1 < n ? chars[i + 1] : nil + if ch == "`" && !inBold && !inStrike { inCode.toggle(); i += 1; continue } + if inCode { i += 1; continue } + if ch == "*" && next == "*" { inBold.toggle(); i += 2; continue } + if ch == "~" && next == "~" { inStrike.toggle(); i += 2; continue } + if ch == "[" && next == "[" { bracketDepth += 1; i += 2; continue } + if ch == "]" && next == "]" && bracketDepth> 0 { bracketDepth -= 1; i += 2; continue } + if ch == "[" && bracketDepth == 0 { bracketDepth = 1; i += 1; continue } + if ch == "]" && bracketDepth> 0 && next != "]" { bracketDepth = 0 - let next = text.index(after: i) - if next < text.endIndex && text[next] == "(" { + if next == "(" { parenDepth = 1 - i = text.index(i, offsetBy: 2); continue + i += 2; continue } - i = text.index(after: i); continue + i += 1; continue } - if ch == ")" && parenDepth> 0 { parenDepth -= 1; i = text.index(after: i); continue } + if ch == ")" && parenDepth> 0 { parenDepth -= 1; i += 1; continue } if !inBold && !inStrike && bracketDepth == 0 && parenDepth == 0 { - for sep in separators { - let sepLen = sep.count - if text.distance(from: i, to: text.endIndex)>= sepLen, - text[i ..< text.index(i, offsetBy: sepLen)] == sep { - return i - } + for sep in separators where matches(chars, at: i, sep) { + return (i, sep.count) } } - i = text.index(after: i) + i += 1 } return nil } + + private static func matches(_ chars: [Character], at i: Int, _ sep: [Character]) -> Bool { + guard i + sep.count <= chars.count else { return false } + for k in 0 ..< sep.count where chars[i + k] != sep[k] { + return false + } + return true + } } diff --git a/Scout/ActionItems/Views/GitHubRefLinkifier.swift b/Scout/ActionItems/Views/GitHubRefLinkifier.swift index 35ef09c..79f5067 100644 --- a/Scout/ActionItems/Views/GitHubRefLinkifier.swift +++ b/Scout/ActionItems/Views/GitHubRefLinkifier.swift @@ -48,6 +48,20 @@ enum GitHubRefLinkifier { pattern: #"github\.com/([A-Za-z0-9][\w.-]*/[A-Za-z0-9][\w.-]*)"# ) + /// Whether `s` holds a `#` immediately followed by an ASCII digit — the + /// necessary condition for `refRe` to match anything. One byte pass, no + /// allocation, no grapheme breaking. + static func containsHashDigit(_ s: String) -> Bool { + var previousWasHash = false + for byte in s.utf8 { + if previousWasHash, byte>= UInt8(ascii: "0"), byte <= UInt8(ascii: "9") { + return true + } + previousWasHash = byte == UInt8(ascii: "#") + } + return false + } + /// Spans that must not be rewritten: markdown links, wikilinks, inline code. private static let protectedRes: [NSRegularExpression] = [ #"\[\[[^\]]*\]\]"#, // [[wikilink]] / [[target|alias]] @@ -56,6 +70,19 @@ enum GitHubRefLinkifier { ].map { try! NSRegularExpression(pattern: 0ドル) } static func linkify(_ s: String) -> String { + // Both branches of `refRe` need a `#` immediately followed by a digit + // (`owner/repo#123` or a bare `#123`), so anything else cannot produce + // a single rewrite — yet the scans below (three protected-range regexes + // plus two repo-inference regexes) ran over every string regardless. + // This sits on each `InlineMarkdownText` cache miss, i.e. the + // scroll-visible path. + // + // Testing for a bare `#` is not enough here: Scout vaults are dense + // with `[#TAG]` mnemonics and `#channel` names, so 1,089 of 2,307 + // rendered strings on a real day contain a `#` and only a handful carry + // an issue ref. Requiring the digit takes the skip rate from 53% to 96% + // and this function from 129 ms to 12 ms over that set. + guard Self.containsHashDigit(s) else { return s } let ns = s as NSString let full = NSRange(location: 0, length: ns.length) diff --git a/Scout/ActionItems/Views/InlineMarkdownText.swift b/Scout/ActionItems/Views/InlineMarkdownText.swift index c663fdb..ccf33a9 100644 --- a/Scout/ActionItems/Views/InlineMarkdownText.swift +++ b/Scout/ActionItems/Views/InlineMarkdownText.swift @@ -76,14 +76,32 @@ struct InlineMarkdownText: View { /// that rebuilding it per body evaluation visibly stalls scrolling through /// a full day of cards. Keys are the raw subject/body strings, which are /// stable across parses for the same task text. - private static var cache: [String: AttributedString] = [:] - private static let cacheCap = 2000 + private struct CacheEntry { + let value: AttributedString + /// Logical clock tick of the last read or write — the LRU ordering. + var lastUsed: UInt64 + } + + private static var cache: [String: CacheEntry] = [:] + private static var clock: UInt64 = 0 + + /// Upper bound on cached renderings. + /// + /// A real day's first paint touches ~935 unique strings (~443 KB of source + /// text), and expanding cards, searching, or switching days pushes the + /// working set several thousand higher. `var` only so tests can shrink it; + /// nothing in the app mutates it. MainActor-isolated like the cache itself. + static var cacheCap = 6000 /// Internal rather than private so tests can assert on the rendered runs — /// that a tag survives the markdown parse as a `scout-tag://` link and /// carries the chip attributes. Pure function; no other caller. static func attributedString(for raw: String) -> AttributedString { - if let hit = cache[raw] { return hit } + clock &+= 1 + if let hit = cache[raw] { + cache[raw]?.lastUsed = clock + return hit.value + } // Tags first: once a tag is a `[label](scout-tag://...)` link, the GitHub // linkifier's protected ranges cover it. The two can't collide on the // same token anyway — a tag needs a letter, a GitHub ref is all digits @@ -100,18 +118,43 @@ struct InlineMarkdownText: View { var computed = (try? AttributedString(markdown: rewritten, options: options)) ?? AttributedString(rewritten) styleTagChips(&computed) - if cache.count>= cacheCap { - // Evict an arbitrary half rather than flushing everything: a full - // clear at the cap means the very next render pass re-parses every - // visible string — the stall this cache exists to prevent. - for key in Array(cache.keys.prefix(cacheCap / 2)) { - cache.removeValue(forKey: key) - } - } - cache[raw] = computed + if cache.count>= cacheCap { evictColdest() } + cache[raw] = CacheEntry(value: computed, lastUsed: clock) return computed } + /// Drop the coldest quarter, oldest first. + /// + /// The previous policy evicted `cache.keys.prefix(cap/2)` — *arbitrary* + /// dictionary order — so the strings currently on screen were as likely to + /// be thrown out as anything else. Past the cap that turned into a re-parse + /// treadmill: a warm pass over a 2,398-string working set measured ~320 ms + /// against 12 ms uncapped, a ×ばつ cliff hit by expanding cards, searching, + /// and switching days. Evicting by last use keeps the visible set resident, + /// so crossing the cap degrades gradually instead of falling off an edge. + /// + /// A quarter at a time so the sort amortizes across many inserts rather + /// than running on every one past the cap. + private static func evictColdest() { + let excess = cache.count - (cacheCap * 3 / 4) + guard excess> 0 else { return } + let coldest = cache + .sorted { 0ドル.value.lastUsed < 1ドル.value.lastUsed } + .prefix(excess) + for (key, _) in coldest { + cache.removeValue(forKey: key) + } + } + + // MARK: - Test seams + + static var cacheCountForTesting: Int { cache.count } + static func cacheContainsForTesting(_ raw: String) -> Bool { cache[raw] != nil } + static func resetCacheForTesting() { + cache.removeAll() + clock = 0 + } + /// Give every `scout-tag://` run the chip treatment: accent ink on an /// accent wash, in a slightly smaller monospace face so a tag reads as an /// object rather than prose. diff --git a/ScoutTests/ActionItems/GitHubRefLinkifierTests.swift b/ScoutTests/ActionItems/GitHubRefLinkifierTests.swift index a8b68b3..ba7c67b 100644 --- a/ScoutTests/ActionItems/GitHubRefLinkifierTests.swift +++ b/ScoutTests/ActionItems/GitHubRefLinkifierTests.swift @@ -87,3 +87,76 @@ struct GitHubRefLinkifierTests { #expect(GitHubRefLinkifier.linkify(input) == input) } } + +/// The fast-path guard added for the render hot path. Both branches of `refRe` +/// need a `#` immediately followed by a digit, so anything else must come back +/// byte-identical — if that ever stops holding, the guard would silently drop +/// rewrites rather than fail loudly, so pin the invariant it rests on. +@Suite("GitHubRefLinkifier — hash-digit fast path") +struct GitHubRefLinkifierFastPathTests { + @Test("Strings with no #digit are returned unchanged") + func noHashDigitRoundTrips() { + for s in [ + "", + "Plain prose with no refs at all", + "example-org/scout is the repo", + "github.com/example-org/scout without a ref", + "[[people/alex]] and `code` and [label](https://example.com/a)", + "PROJ-1234 — Priya merged it", + "Numbers 1234 and 5678 but no hash", + "🚧 emoji and — an em dash", + // The case that makes the digit requirement worth having: Scout + // vaults are full of these, and none is a GitHub ref. + "[#PRGREISS] Prague reissue is still open", + "Discussed in #tmp-cuesta-star with [[people/priya]]", + "[#AI3026] and [#RSM] in one line", + "A trailing hash # and a lone #", + // Digit-leading tags DO clear the guard — `#5` is a hash followed + // by a digit — so they still pay for the regex scan. They must + // nonetheless come back untouched: `[#5864M]` is not `#5864` (the + // trailing `M` defeats refRe's `\b`), and it is bracket-protected. + "[#5864M] the reissue coupon", + ] { + #expect(GitHubRefLinkifier.linkify(s) == s, "should be untouched: \(s)") + } + } + + @Test("Alpha-leading tags and channel names skip the regex scan entirely") + func guardSkipsTagsAndChannels() { + for s in [ + "[#PRGREISS] Prague reissue is still open", + "Discussed in #tmp-cuesta-star with [[people/priya]]", + "[#AI3026] and [#RSM] in one line", + "A trailing hash # and a lone #", + "no hash at all", + ] { + #expect(!GitHubRefLinkifier.containsHashDigit(s), "guard should skip: \(s)") + } + } + + @Test("The guard admits every shape refRe can match") + func guardAdmitsRealRefs() { + for s in [ + "#42", + "see #42 in example-org/scout", + "example-org/scout#42", + "trailing ref example-org/scout#7", + "[#TAG] alongside a real #99 ref", + ] { + #expect(GitHubRefLinkifier.containsHashDigit(s), "guard must not skip: \(s)") + } + } + + @Test("Strings with a # still linkify") + func hashStillLinkifies() { + // Qualified ref — carries its own repo. + #expect(GitHubRefLinkifier.linkify("example-org/scout#42") + .contains("https://github.com/example-org/scout/issues/42")) + // Bare ref — only linkifies once a single repo can be inferred from the + // same string, so give it one. (Without a repo it stays plain; that's + // the existing `leavesBareRefsAloneWithoutARepo` case, not a fast-path + // regression.) + #expect(GitHubRefLinkifier.linkify("see #42 in example-org/scout") + .contains("https://github.com/example-org/scout/issues/42")) + } +} diff --git a/ScoutTests/ActionItems/InlineMarkdownCacheTests.swift b/ScoutTests/ActionItems/InlineMarkdownCacheTests.swift new file mode 100644 index 0000000..536551d --- /dev/null +++ b/ScoutTests/ActionItems/InlineMarkdownCacheTests.swift @@ -0,0 +1,84 @@ +import Testing +import Foundation +@testable import Scout + +// @MainActor is load-bearing here for the same reason it is on +// `InlineMarkdownTextTests` — the static cache is MainActor-isolated in the app +// module, and an unannotated suite races the test host's main-thread renders. +@MainActor +@Suite("Inline markdown cache — LRU eviction") +struct InlineMarkdownCacheTests { + + /// Shrink the cap so the test drives eviction in milliseconds instead of + /// parsing thousands of strings, and leave it as found. + private func withCap(_ cap: Int, _ body: () -> Void) { + let original = InlineMarkdownText.cacheCap + InlineMarkdownText.resetCacheForTesting() + InlineMarkdownText.cacheCap = cap + defer { + InlineMarkdownText.cacheCap = original + InlineMarkdownText.resetCacheForTesting() + } + body() + } + + @Test("A repeatedly used entry survives eviction of a much larger cold set") + func hotEntrySurvivesEviction() { + withCap(50) { + let hot = "the hot string PROJ-1234 **bold** and `code`" + _ = InlineMarkdownText.attributedString(for: hot) + + // Ten times the cap in cold traffic, with the hot string touched + // regularly throughout. The old policy evicted `cache.keys.prefix` + // — arbitrary dictionary order — so the hot entry was as likely to + // be dropped as any other, which is what made the cliff a cliff. + for i in 0 ..< 500 { + _ = InlineMarkdownText.attributedString(for: "cold entry number \(i)") + if i % 5 == 0 { + _ = InlineMarkdownText.attributedString(for: hot) + } + } + + #expect(InlineMarkdownText.cacheContainsForTesting(hot)) + } + } + + @Test("The cache stays bounded by its cap") + func staysBounded() { + withCap(50) { + for i in 0 ..< 500 { + _ = InlineMarkdownText.attributedString(for: "entry \(i)") + } + #expect(InlineMarkdownText.cacheCountForTesting <= 50) + } + } + + @Test("Eviction drops the coldest entries, not the most recent") + func evictsColdestFirst() { + withCap(20) { + // Fill past the cap with a known access order: `recent` is touched + // last, `ancient` first and never again. + let ancient = "ancient entry" + _ = InlineMarkdownText.attributedString(for: ancient) + for i in 0 ..< 60 { + _ = InlineMarkdownText.attributedString(for: "filler \(i)") + } + let recent = "recent entry" + _ = InlineMarkdownText.attributedString(for: recent) + + #expect(InlineMarkdownText.cacheContainsForTesting(recent)) + #expect(!InlineMarkdownText.cacheContainsForTesting(ancient)) + } + } + + @Test("A cache hit returns the same rendering as a cold parse") + func hitMatchesMiss() { + withCap(50) { + let s = "**Bold** with [[people/alex]] and example-org/scout#42" + let cold = InlineMarkdownText.attributedString(for: s) + let warm = InlineMarkdownText.attributedString(for: s) + #expect(String(cold.characters) == String(warm.characters)) + #expect(cold == warm) + } + } +} diff --git a/ScoutTests/ActionItems/SplitSubjectBodyTests.swift b/ScoutTests/ActionItems/SplitSubjectBodyTests.swift new file mode 100644 index 0000000..1f116a3 --- /dev/null +++ b/ScoutTests/ActionItems/SplitSubjectBodyTests.swift @@ -0,0 +1,272 @@ +import Testing +import Foundation +@testable import Scout + +/// `splitSubjectBody` was quadratic: `firstSeparatorOutsideTokens` called +/// `text.distance(from:to:)` — itself O(n) — three times per character, so cost +/// grew with the square of the line length. On a real day's file that one +/// function was 465 ms at `-O`, 68% of the whole 830 ms parse, and task lines +/// run to 9,685 characters. +/// +/// The rewrite walks a `[Character]` array with integer indices. Behavior must +/// not move a millimetre: `parser-corpus.json` is byte-identical across three +/// repos and only carries 6 entries with a body, so the safety net here is a +/// differential test against the original implementation, kept verbatim below. +@Suite("splitSubjectBody — separator scanning") +struct SplitSubjectBodyTests { + + // MARK: - Curated cases + + @Test("Splits on each dash separator, outside tokens") + func splitsOnDashes() { + #expect(ActionItemsParser.splitSubjectBody("Ship it — by Friday").0 == "Ship it") + #expect(ActionItemsParser.splitSubjectBody("Ship it — by Friday").1 == "by Friday") + #expect(ActionItemsParser.splitSubjectBody("Ship it – by Friday").1 == "by Friday") + #expect(ActionItemsParser.splitSubjectBody("Ship it - by Friday").1 == "by Friday") + } + + @Test("Falls back to a colon separator when no dash is present") + func colonFallback() { + let (s, b) = ActionItemsParser.splitSubjectBody("Blocked: waiting on Priya") + #expect(s == "Blocked") + #expect(b == "waiting on Priya") + } + + @Test("A separator inside a token is not a split point") + func separatorsInsideTokensAreIgnored() { + // Each of these has its only ` — ` inside a token, so nothing splits. + for raw in [ + "**Bold — inner** and the rest", + "`code — inner` and the rest", + "~~struck — inner~~ and the rest", + "[[wiki — inner]] and the rest", + "[label — inner](https://example.com/x) and the rest", + ] { + let (s, b) = ActionItemsParser.splitSubjectBody(raw) + #expect(b == "", "should not split: \(raw)") + #expect(s == raw, "subject should be the whole line: \(raw)") + } + } + + @Test("Splits after a token closes") + func splitsAfterTokenCloses() { + let (s, b) = ActionItemsParser.splitSubjectBody("**PROJ-1234 shipped** — Priya merged it") + #expect(s == "**PROJ-1234 shipped**") + #expect(b == "Priya merged it") + } + + @Test("No separator leaves the line intact with an empty body") + func noSeparator() { + let (s, b) = ActionItemsParser.splitSubjectBody("Just a subject") + #expect(s == "Just a subject") + #expect(b == "") + } + + @Test("Grapheme clusters are not split mid-character") + func graphemeSafety() { + let (s, b) = ActionItemsParser.splitSubjectBody("🚧 Blocked 👩‍👩‍👧‍👦 — see the thread") + #expect(s == "🚧 Blocked 👩‍👩‍👧‍👦") + #expect(b == "see the thread") + } + + // MARK: - Differential against the original implementation + + @Test("Agrees with the original implementation across generated inputs") + func differentialAgainstReference() { + var checked = 0 + for raw in Self.generatedCorpus() { + let new = ActionItemsParser.splitSubjectBody(raw) + let old = Self.referenceSplitSubjectBody(raw) + #expect(new.0 == old.0, "subject differs for: \(raw)") + #expect(new.1 == old.1, "body differs for: \(raw)") + checked += 1 + } + // Guard against the generator silently collapsing to nothing. + #expect(checked> 1500) + } + + /// Fragments chosen to drive every branch of the scanner: each token kind + /// both closed and unclosed, each separator, and separators sitting inside + /// and outside tokens. + static func generatedCorpus() -> [String] { + let fragments = [ + "plain text", + "**bold**", + "**bold — dash**", + "**unclosed bold", + "`code`", + "`code — dash`", + "`unclosed code", + "~~struck~~", + "~~struck — dash~~", + "[[wikilink]]", + "[[wiki|alias]]", + "[[wiki — dash]]", + "[label](https://example.com/a)", + "[label — dash](https://example.com/a)", + "[unclosed label", + "]stray close", + ")stray paren", + "PROJ-1234", + "🚧 emoji", + "colon: inside", + ] + let separators = ["", " — ", " – ", " - ", ": ", " ", "—", "-"] + + var out: [String] = [] + for a in fragments { + for sep in separators { + for b in fragments { + out.append("\(a)\(sep)\(b)") + } + } + } + // A few hand-built shapes the product doesn't reach. + out.append(contentsOf: [ + "", + " ", + " — ", + "— leading", + "trailing —", + "a — b — c", + "a: b: c", + "**a** — **b** — **c**", + "[[a]] — [[b]]", + "`a` — `b`", + "[a](u) — [b](v)", + "**[[nested]] — after**", + "[[**nested bold** — x]] — after", + ]) + return out + } + + // MARK: - The original, kept verbatim as the differential reference + // + // Do not "clean up" or re-optimize this copy — its whole value is being the + // pre-rewrite behavior. If a corpus entry ever legitimately changes, change + // the production code and this reference together, deliberately. + + static func referenceSplitSubjectBody(_ rest: String) -> (String, String) { + let separators = [" — ", " – ", " - "] + if let idx = referenceFirstSeparatorOutsideTokens(in: rest, separators: separators) { + for sep in separators { + let sepLen = sep.count + if rest.distance(from: idx, to: rest.endIndex)>= sepLen, + rest[idx ..< rest.index(idx, offsetBy: sepLen)] == sep { + return ( + String(rest[.. String.Index? { + var inBold = false, inStrike = false, inCode = false + var bracketDepth = 0, parenDepth = 0 + var i = text.startIndex + while i < text.endIndex { + let rem = text[i...] + let two = rem.prefix(2) + let ch = text[i] + if ch == "`" && !inBold && !inStrike { inCode.toggle(); i = text.index(after: i); continue } + if inCode { i = text.index(after: i); continue } + if two == "**" { inBold.toggle(); i = text.index(i, offsetBy: 2); continue } + if two == "~~" { inStrike.toggle(); i = text.index(i, offsetBy: 2); continue } + if two == "[[" { bracketDepth += 1; i = text.index(i, offsetBy: 2); continue } + if two == "]]" && bracketDepth> 0 { bracketDepth -= 1; i = text.index(i, offsetBy: 2); continue } + if ch == "[" && bracketDepth == 0 { bracketDepth = 1; i = text.index(after: i); continue } + if ch == "]" && bracketDepth> 0 && two != "]]" { + bracketDepth = 0 + let next = text.index(after: i) + if next < text.endIndex && text[next] == "(" { + parenDepth = 1 + i = text.index(i, offsetBy: 2); continue + } + i = text.index(after: i); continue + } + if ch == ")" && parenDepth> 0 { parenDepth -= 1; i = text.index(after: i); continue } + if !inBold && !inStrike && bracketDepth == 0 && parenDepth == 0 { + for sep in separators { + let sepLen = sep.count + if text.distance(from: i, to: text.endIndex)>= sepLen, + text[i ..< text.index(i, offsetBy: sepLen)] == sep { + return i + } + } + } + i = text.index(after: i) + } + return nil + } +} + +/// The cost curve, not a wall-clock budget. +/// +/// A hard millisecond bound would be flaky on a loaded machine, so this asserts +/// the *shape*: quadratic cost quadruples when the input doubles, linear cost +/// roughly doubles. The threshold sits between the two. +@Suite("splitSubjectBody — cost grows linearly") +struct SplitSubjectBodyScalingTests { + + /// A worst-case line: no separator anywhere, so the scanner walks every + /// character, and peppered with tokens so no branch short-circuits. + static func line(chars: Int) -> String { + let unit = "some **bold** and `code` and [[a-wiki-link]] plus prose " + var s = "" + while s.count < chars { s += unit } + return String(s.prefix(chars)) + } + + static func medianMS(of block: () -> Void, runs: Int = 5) -> Double { + var samples: [Double] = [] + for _ in 0..×ばつ) at these very sizes. + let ratio = tLong / max(tShort, 0.0001) + #expect(ratio < 3.0, "cost ratio 8k/4k was \(ratio) (short \(tShort) ms, long \(tLong) ms)") + } + + @Test("A pathological line stays far below the quadratic cost") + func longLineIsFast() { + // 16k chars measured 410 ms with the quadratic scan. Linear is ~1 ms; + // 60 ms leaves a wide margin for Debug and CI noise while still being + // unreachable for the old implementation. + let huge = Self.line(chars: 16_000) + _ = ActionItemsParser.splitSubjectBody(huge) + let t = Self.medianMS(of: { _ = ActionItemsParser.splitSubjectBody(huge) }, runs: 3) + #expect(t < 60, "16k-char line took \(t) ms") + } +} From 01ca9fe61e0d6af3da0307f76e0d6468f26254be Mon Sep 17 00:00:00 2001 From: jordanrburger Date: 2026年9月10日 19:03:07 -0500 Subject: [PATCH 2/2] review(#100): anonymize lifted fixtures, align refRe with its guard - Replace vault-lifted test literals (#tmp-cuesta-star, [#PRGREISS] Prague reissue) with synthetic stand-ins per CLAUDE.md. - refRe now matches [0-9] rather than \d so containsHashDigit really is its necessary condition; add a fullwidth-digit regression test. - Fix two stale comments (the cache function is no longer pure; [#5864M] is not bracket-protected) and a wrong test-name cross-reference. Co-Authored-By: Claude Fable 5.1 --- .../Views/GitHubRefLinkifier.swift | 7 +++--- .../Views/InlineMarkdownText.swift | 4 ++- .../ActionItems/GitHubRefLinkifierTests.swift | 25 +++++++++++++------ 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/Scout/ActionItems/Views/GitHubRefLinkifier.swift b/Scout/ActionItems/Views/GitHubRefLinkifier.swift index 79f5067..ebe5187 100644 --- a/Scout/ActionItems/Views/GitHubRefLinkifier.swift +++ b/Scout/ActionItems/Views/GitHubRefLinkifier.swift @@ -31,7 +31,7 @@ enum GitHubRefLinkifier { /// the same `#N`. Group 1/2 = qualified owner/repo + number; group 3 = /// bare number. private static let refRe = try! NSRegularExpression( - pattern: #"(? Bool { var previousWasHash = false for byte in s.utf8 { diff --git a/Scout/ActionItems/Views/InlineMarkdownText.swift b/Scout/ActionItems/Views/InlineMarkdownText.swift index ccf33a9..91f41e4 100644 --- a/Scout/ActionItems/Views/InlineMarkdownText.swift +++ b/Scout/ActionItems/Views/InlineMarkdownText.swift @@ -95,7 +95,9 @@ struct InlineMarkdownText: View { /// Internal rather than private so tests can assert on the rendered runs — /// that a tag survives the markdown parse as a `scout-tag://` link and - /// carries the chip attributes. Pure function; no other caller. + /// carries the chip attributes. Not pure: every call ticks the LRU clock + /// and may insert or evict, so it stays main-actor-bound. `init` is the + /// only production caller. static func attributedString(for raw: String) -> AttributedString { clock &+= 1 if let hit = cache[raw] { diff --git a/ScoutTests/ActionItems/GitHubRefLinkifierTests.swift b/ScoutTests/ActionItems/GitHubRefLinkifierTests.swift index ba7c67b..1502331 100644 --- a/ScoutTests/ActionItems/GitHubRefLinkifierTests.swift +++ b/ScoutTests/ActionItems/GitHubRefLinkifierTests.swift @@ -107,15 +107,15 @@ struct GitHubRefLinkifierFastPathTests { "🚧 emoji and — an em dash", // The case that makes the digit requirement worth having: Scout // vaults are full of these, and none is a GitHub ref. - "[#PRGREISS] Prague reissue is still open", - "Discussed in #tmp-cuesta-star with [[people/priya]]", + "[#DEMOTAG] the demo launch is still open", + "Discussed in #tmp-demo-sync with [[people/priya]]", "[#AI3026] and [#RSM] in one line", "A trailing hash # and a lone #", // Digit-leading tags DO clear the guard — `#5` is a hash followed // by a digit — so they still pay for the regex scan. They must // nonetheless come back untouched: `[#5864M]` is not `#5864` (the - // trailing `M` defeats refRe's `\b`), and it is bracket-protected. - "[#5864M] the reissue coupon", + // trailing `M` defeats refRe's `\b`). + "[#5864M] the demo coupon", ] { #expect(GitHubRefLinkifier.linkify(s) == s, "should be untouched: \(s)") } @@ -124,8 +124,8 @@ struct GitHubRefLinkifierFastPathTests { @Test("Alpha-leading tags and channel names skip the regex scan entirely") func guardSkipsTagsAndChannels() { for s in [ - "[#PRGREISS] Prague reissue is still open", - "Discussed in #tmp-cuesta-star with [[people/priya]]", + "[#DEMOTAG] the demo launch is still open", + "Discussed in #tmp-demo-sync with [[people/priya]]", "[#AI3026] and [#RSM] in one line", "A trailing hash # and a lone #", "no hash at all", @@ -154,9 +154,20 @@ struct GitHubRefLinkifierFastPathTests { .contains("https://github.com/example-org/scout/issues/42")) // Bare ref — only linkifies once a single repo can be inferred from the // same string, so give it one. (Without a repo it stays plain; that's - // the existing `leavesBareRefsAloneWithoutARepo` case, not a fast-path + // the existing `leavesBareRefsPlainWhenNoRepo` case, not a fast-path // regression.) #expect(GitHubRefLinkifier.linkify("see #42 in example-org/scout") .contains("https://github.com/example-org/scout/issues/42")) } + + @Test("Non-ASCII digits are not issue numbers, guard or no guard") + func nonASCIIDigitsAreNotRefs() { + // `refRe` spells its digits as `[0-9]` so it agrees with + // `containsHashDigit` by construction. Pair the fullwidth ref with an + // ASCII one so the guard admits the string and the regex really runs. + let out = GitHubRefLinkifier.linkify("example-org/scout#42 and #1") + #expect(!out.contains("issues/42"), "fullwidth digits linkified: \(out)") + #expect(out.contains("https://github.com/example-org/scout/issues/1")) + #expect(!GitHubRefLinkifier.containsHashDigit("example-org/scout#42")) + } }

AltStyle によって変換されたページ (->オリジナル) /