Speed up bulk Find & Replace with a native single-session string replace - #1065
Speed up bulk Find & Replace with a native single-session string replace#1065johnml1135 wants to merge 3 commits into
Conversation
Compute each bulk preview value once. Replace all matches through one native ICU search session. Preserve rich text, Unicode collation, and legacy fallback behavior. Record measured gains and discarded experiments.
Guard the outer bulk-edit undo task with try/finally so a native ReplaceAllIn fault mid-batch cannot leave it unterminated. Share one computed value between OkToChange and TryGetNewValue in BulkCopyMethod and TransduceMethod instead of computing twice, scoped to stay correct across separate preview/apply calls. Add native ReplaceAllIn coverage for ORC, writing-system restriction, case/diacritics, and canonical equivalence. Harden the NFD-normalization skip check with non-Latin script and rich-run test cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1065 +/- ##
==========================================
+ Coverage 37.91% 38.10% +0.19%
==========================================
Files 1499 1499
Lines 350117 350276 +159
Branches 40233 40235 +2
==========================================
+ Hits 132747 133480 +733
+ Misses 188043 187521 -522
+ Partials 29327 29275 -52
🚀 New features to boost your workflow:
|
Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md was a one-time investigation log, not durable guidance. Its measurements, rejected approaches, and follow-up items now live in the PR body. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
7cd1917 to
30baeb4
Compare
jasonleenaylor
left a comment
jasonleenaylor
left a comment
There was a problem hiding this comment.
The ABI work here is done right, and I want to say that before the list. Versioning a
published COM surface by appending a new derived interface with its own GUID, adding it
to the coclass, extending QueryInterface with CSupportErrorInfo2, and detecting it
once by cast on the managed side is exactly how you extend Views without breaking
anyone — and verifying it against the generated MIDL header rather than the .idh
source is the right paranoia. The four reverted micro-optimizations, each documented with
why it was unsafe, are the most useful part of the body; the ICU-collation reasoning
behind dropping the memcmp shortcut is the kind of thing that saves the next person a
week.
I also checked the refactor's blast radius before writing this, and it is smaller than it
looks. Splitting FindInAlgorithmBase::Run and rerouting both forward Search branches
through SearchNext sits on the hot path of FwFindReplaceDlg and FindCollectorEnv,
which nothing in the verification list exercises directly — but the twenty existing
native tests in TestVwPattern.h (testSimpleSearch, testRealSearch,
testRegExpSearch, testMatchingWs, testSurrogatePairSearch, testORCSearch,
testReorderingDiactritics and the rest) all run through FindIn -> Run() ->
Search(), so the refactored path is pinned by construction. That is the coverage that
matters, and it is green. No action needed; I am recording it so nobody re-raises it.
What needs to change.
1. Roll back the batch on failure; do not commit it.
You correctly spotted a real bug: Doit(IEnumerable<int>, ProgressState) previously
called BeginUndoTask and EndUndoTask with nothing in between to guarantee the close,
so a throw left the undo task dangling open. That needed fixing.
But EndUndoTask() in a bare finally (BulkEditBar.cs:4839-4862) fixes it the weaker
way: when row 30,000 of 50,000 throws, the 29,999 already-applied rows are committed
as a completed undoable task and the exception then propagates. The user is left with a
half-applied bulk edit that undo treats as a finished operation.
This file has already answered this question four times —
UndoableUnitOfWorkHelper.Do(...) at BulkEditBar.cs:2147, :5976, :6469, :6999 —
and it rolls back on exception. FwFindReplaceDlg.cs:1211-1215 does the same explicitly,
setting undoHelper.RollBack = false only after the work succeeds. Please use the
helper.
The divergence is what I want to flag, more than the line itself. The reason this file
has one hand-rolled undo block and four helper calls is that each new piece of work in
here brings its own. That is the same drift that left the repo with two replace-all
engines (see item 3), and it is worth resisting on principle even where the behaviour
difference looks unlikely to bite.
Doit_OuterLoop_EndsUndoTaskWhenAnItemThrows
(BulkEditBarTests.cs:2536-2552) asserts only CurrentDepth == 0, which is true under
either policy — so it cannot tell commit from rollback and would not notice a
regression either way. Please assert the data: after the throw, the rows processed before
the failure are not present.
2. Take an IVwSearchKiller * on ReplaceAllIn now, even if every caller passes NULL.
Your own argument for creating IVwPattern2 rather than adding to IVwPattern is that a
published interface is permanent and changing it forces every implementation and consumer
to move in lockstep. That reasoning applies one level down. ReplaceAllIn hard-codes
NULL into both algorithm constructions (VwPattern.cpp:1367, :1373), so the entire
per-string replace is an uncancellable native call — and if cancellation is ever wanted,
the fix is IVwPattern3, for exactly the reason you gave.
This is not a regression: the old managed loop passed null too. It is a permanent shape
being decided by omission rather than on purpose, on a brand-new interface, while
UseRegularExpressions is reachable from the UI and catastrophic backtracking is a real
possibility on a 50,000-row table. An unused parameter costs one line today. A third
interface does not.
3. ReplaceAllIn breaks the COM out-parameter contract, and a new test asserts the
breakage.
VwPattern.cpp:1343-1353 validates pcMatches first, so a caller passing a null
pcMatches gets E_POINTER with *pptssResult left holding whatever it held on entry.
COM requires every [out] parameter be zeroed on failure.
testReplaceAllInValidatesOutputsAndRanges in TestVwPattern.h currently pins that:
unitpp::assert_true("result is untouched when first output is null",
ptssRawResult == reinterpret_cast<ITsString *>(1));
Every other failure case in that same test correctly asserts both outputs are cleared.
Validate all pointers before writing any of them, then let this case assert NULL like
its siblings.
4. Extract the zero-length-match fixup instead of copying it.
VwPattern.cpp:1291-1299 reproduces :1623-1638 character-for-character in logic — the
ichMin == ichLim test, the !m_fUseRegularExpressions || !m_stuCompiled.Equals(L"^")
guard, the +1 bump, the clamp. The original carries three explanatory comments and the
LT-6707 reference; the copy carries none. Two independent copies of a subtle Unicode
edge case will drift, and the commenting standard names LT-##### as the sanctioned
durable pointer precisely so this reasoning survives. A private helper on VwPattern
called from both sites fixes it.
5. Use ITsString.get_IsNormalizedForm.
BulkEditBar.cs:5188-5197 marshals tssResult.Text out as a BSTR to feed
CustomIcu.GetIcuNormalizer(...).IsNormalized(text). The interface you already hold
answers this directly — get_IsNormalizedForm(FwNormalizationMode), declared natively at
Src/views/lib/TsString.h:547 and used in this solution at
ConfiguredXHTMLGeneratorTests.cs:10436 and LcmWordGeneratorTests.cs:707. Reaching for
CustomIcu is correct SIL-library reuse in general; here a cheaper, closer API was in
hand.
While you are in there: the old code called get_NormalizedForm unconditionally, and the
new skip path returns the builder's string untouched when the text is already NFD. If
get_NormalizedForm also normalizes run segmentation, that changes run structure for
already-NFD input. FakeDoit_MatchesOracleForDecoratorBackedUnnormalizedRichValue
compares run-level equality against a get_NormalizedForm oracle, which is reassuring,
but it is one case.
6. Justify the numbers on a Release build, and narrow the regex claim.
The measured 29.1% / 23-27% figures are from a Debug build. For unoptimized C++ over ICU,
the ratio of per-call setup to per-match search cost is exactly what optimization changes
most, so Debug percentages are not a prediction of shipped behaviour. Please re-measure in
Release and quote those.
I am not asking you to commit the measurement log, and I am not asking for a durable
performance test — unless you can see one that would survive the Avalonia refactoring
intact. A benchmark fixture that gets rewritten or deleted in six months is worse than
none.
Separately, "one native ICU search session" is not true of the regex path.
RegexMatcher::find(start, status) resets the matcher and re-scans from start on every
call, so RegExFindInAlgorithm::SearchNext (VwPattern.cpp:1613-1625) still restarts per
match; what it saves there is the UnicodeString copy, the VwStringTextSource
construction and the FetchSearch round trip. Only the non-regex SearchNext
(:1583-1608), advancing m_piter->next(), is a genuine single session. Worth stating
precisely, because the structural argument for the win is strong on its own — preview
really does drop from two searches per row to one, and it is worth noting the probe you
removed was the cheaper of the two, which is why ~29% rather than ~50% is the honest
shape.
7. Test placement and the fixture that already covers this.
Both new managed files land in xWorksTests/Avalonia/Performance/. Nothing in either
touches Avalonia, and that tree otherwise holds only Composer/, Hosting/, Plugins/.
Please move them.
More substantively: xWorksTests/Search/BulkEditReplaceCharacterizationTests.cs already
exists, on the same base class, and its summary says it "Records the current preview and
apply behavior of bulk replacement over citation forms." That is the fixture whose entire
purpose is pinning the behaviour this PR changes. It is not in the verification list, and
the new preview tests were written from scratch in a new directory rather than extending
it. Either extend it or say why it is superseded — but a characterization fixture that
nobody consults during a semantic change is not doing its job.
8. VwPatternReplacementTests needs [Apartment(ApartmentState.STA)].
It calls VwPatternClass.Create() and VwStringTextSourceClass.Create(), both registered
threadingModel="Apartment". xWorksTests has no assembly-level apartment setting, and
the two sibling fixtures this PR adds in ReplaceWithMethodTests.cs both declare it. This
one is the odd fixture out.
9. Document the new null contract on NewValue.
TryGetNewValue returns newValue != null (BulkEditBar.cs:4912-4924), so null from
the abstract NewValue now means "skip this row" for every subclass. Today's four are
safe — BulkCopyMethod, TransduceMethod and ClearMethod all fall back to
TsStringUtils.EmptyString, and only ReplaceWithMethod returns null — so this is not
a live bug. But protected abstract ITsString NewValue(int hvo); carries no doc comment
saying so, and the next subclass author has no way to learn it.
10. Comments.
NewValueCached(BulkEditBar.cs:4926-4936) narrates its callers and their mechanism:
"reusing a value anOkToChangeoverride already computed ... The cache is cleared at
the end of everyTryGetNewValuecall." A member's summary states its own contract
only. One sentence does it: returnsNewValue(hvo), computing it at most once per hvo
until the cache is cleared.- The comment above
testReplaceAllInRespectsMatchOldWritingSysteminTestVwPattern.h
references another test by name, which the standard bans outright, and runs to roughly
235 characters against a 200-character budget. testReplaceAllInReplaceCharPrecedingFinalORC_TE4727(~228 chars) and the U+AC00
comment inReplaceWithMethodTests.cs(~215 chars) are both over budget; the first also
opens by restating the test's own name.// The first match extends past the end of our range.survives verbatim into
FindInAlgorithm::SearchNext(VwPattern.cpp:1601), where it is no longer the first
match.
The Views.idh block on IVwPattern2 is the model for the rest: four short lines,
contract only, and it states the non-obvious part ("Success leaves the pattern in a
terminal no-match state") that the tests then verify.
3 participants
Bulk Find & Replace over string fields (e.g. replacing text across thousands of Citation Forms) now runs each entry's search-and-replace through one native ICU search session instead of repeatedly restarting
FindInper match. Preview also computes each row's result once instead of twice. Both changes are additive: a new optionalIVwPattern2.ReplaceAllInCOM capability, used when available, with the original repeated-FindInpath kept as the fallback when it isn't.The diff crosses the native/managed boundary (a new COM interface) and touches a widely-used feature, so the real question isn't "is it faster" — the measurements below answer that — it's "does the fast path ever produce a different (or unsafe) result than the slow path did, and can a fault in it corrupt an in-progress bulk edit." That's what the checklist below is aimed at.
Where to look:
IVwPattern2is a new GUID appended after the complete, unmodifiedIVwPatternvtable; verified against the generated MIDL header, not just the.idhsource. No default-coclass change.VwPatternReplacementTests.cscross-checks the real nativeReplaceAllInagainst a repeated-FindInoracle: regex, collation/locale tailoring, whole-word, writing-system/style/tag runs, RTL, and combining marks.try/finallyand pinned byDoit_OuterLoop_EndsUndoTaskWhenAnItemThrows.FakeDoit_FallsBackWhenBulkReplacementIsUnavailable); the capability is detected once, not probed per call.Deliberately not here:
ReplaceAllInsearches a raw string and does not reproduceFindIn's VC-aware omission of embedded object-replacement characters (footnote markers etc.) from the pattern span — an architectural difference in what the two APIs search over, not a regression (see accordion).Verification:
./build.ps1(full native + managed): 0 warnings, 0 errors. Managed: 75/75 (ReplaceWithMethodPreviewTests,ReplaceAllInDecoratorCorrectnessTests,BulkEditBarTests,VwPatternReplacementTests). NativeVwPatternsuite: 27/27. The full 309-test native suite also reports all-pass but the process hangs ~5s in an unrelated Uniscribe/Graphite teardown path afterward (reproduced twice, unrelated subsystem, pre-existing). No manual FLEx UI pass was performed.Reading this a year from now — start here
This PR started as a focused perf change (one squashed commit) and picked up a second commit from its own pre-merge review, which found and fixed three real issues before they shipped. The working measurement log that produced the perf numbers below lived at
Docs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.mdon the branch; its conclusions are captured here and the file was deleted rather than merged, since it was a one-time investigation log, not guidance anyone needs to read to change this code correctly.Decisions, and why
Why an optional
IVwPattern2capability instead of changingIVwPattern.IVwPatternis an existing, ABI-relied-upon COM interface. AddingReplaceAllInto it would have required every existing implementation and consumer to change in lockstep. Appending a new interface, detected once via anas/QueryInterface-style cast and cached, gets the perf win without touching the existing contract, and lets a caller that only implementsIVwPatternkeep working unmodified via the repeated-FindInfallback.Why preview and apply are independent evaluations, not a shared cache.
FakeDoit(preview) andDoit(apply) are separate top-level calls, and the design intentionally recomputes on each — a bulk-edit column's preview row and its later apply are allowed to diverge if something else changed the underlying data in between. A per-row cache was added inside a singleTryGetNewValuecall (soBulkCopyMethod/TransduceMethod'sOkToChangeandTryGetNewValueshare one computed value instead of two) but is explicitly cleared at the end of every call, so the preview-then-apply pair still each compute fresh. See "Reversals" below for what happens when that clearing is missing.Why
ReplaceAllIndoesn't omit embedded ORCs the wayFindIncan.FindIncan search through aVwMappedTxtSrc, a view-constructor-aware text source that can skip owned object-replacement characters (e.g. footnote markers) so a pattern can match across one without "seeing" it.ReplaceAllInoperates on a rawITsStringviaTrivialTextSrcand has no such view-aware skip. This wasn't somethingReplaceAllIn's contract ever claimed to do; a bulk-replace call site that needs that omission would need to pass a pre-mapped source, which none currently does.Reversals
The first version of the
OkToChange/TryGetNewValuevalue-sharing cache (see above) did not clear itself between calls — it cached strictly by row ID, with no notion of "this call is done." That collapsedReplaceWithMethod's intentional preview-then-apply double-evaluation down to a single evaluation, since both calls share the same row ID on the same method instance. It brokeFakeDoit_MatchesImmediateApplyAcrossPatternModes(7 of its cases failed, each expecting the bulk-replace call count to reach 2, not 1) on the very next full test run after the "fix" was written. Caught by running the full suite rather than only the newly-added test, fixed by clearing the cache at the end of everyTryGetNewValuecall, and reverified at 75/75.Deferred, and what would unblock it
Homograph-renumber batching (LT-22701): re-sorting and renumbering an entire homograph group happens on every single entry write, even though only the last write in a batch determines the group's final state — measured at roughly 34.6 microseconds/entry, about 17% of a 100%-match bulk operation. Unblocking it needs a liblcm-side design pass covering undo/redo interaction,
PropChanged/notification behavior, cache membership mid-batch, and correct final numbering when multiple entries in the same group are edited in one operation — none of which this PR's scope (FieldWorks-side search/replace) can settle on its own.Paths not taken
memcmpshortcut in the native search path — reverted. ICU collation can equate strings that differ in punctuation and other non-ordinal ways, so a raw byte-compare shortcut produced wrong matches under real collation rules.Evidence
All measurements below are Debug-build, incremental (isolating one change at a time, not a cumulative branch-start-to-finish number), 50,000 real entries, and were taken before this PR's second (review-fix) commit — that commit fixes correctness/robustness issues and adds test coverage, and does not change the measured code paths.
Preview (single-pass) cost, 100% matching CitationForm, one match each: pooled median 72.4 → 51.4 microseconds/entry, a 29.1% reduction (1.41x throughput). Two paired process runs per build.
ReplaceAllInapply cost (fresh processes, one warmup + five timed repetitions each,old old old old-######-style matched values with four matches each):Debug-process variance was material — each independent 50%/100% pair exceeded a 15% self-imposed acceptance gate on its own, which is why the pooled-median figures above are quoted rather than any single run. Real-world gains should track the shape (bigger win at higher match rates) more reliably than the exact percentages.
Native test coverage added in the review-fix commit:
testReplaceAllInReplaceCharPrecedingFinalORC_TE4727,testReplaceAllInRespectsMatchOldWritingSystem,testReplaceAllInWithCaseAndDiacriticsOptions,testReplaceAllInCanonicalEquivalence— re-running scenarios that previously only hadFindIncoverage through the new bulk-session path. FullVwPatternnative suite: 27/27 passing.Preflight review details
Code Review Summary
Branch: table-speedup
Base: main (origin/main, merge-base
7f93348966a22be7fd4f9ef0c2e1cf571281cbcd)Date: 2026-08-14
Review model: Claude Sonnet 5 (Claude Code)
Files changed: 9
Overview
This branch speeds up bulk Find & Replace over string fields. It adds an optional
IVwPattern2COM capability (ReplaceAllIn) that replaces every match in a stringthrough one native ICU search session instead of the previous repeated-
FindInloop,falls back to the old per-match path when the capability isn't available, and computes
each bulk-preview value once instead of twice. Measured gains (Debug, distillation doc):
~29% reduction in preview cost, ~23-27% reduction in
ReplaceAllInapply cost dependingon match rate.
Two independent specialist passes (native/COM/boundary-safety, managed C#/UI) reviewed
the diff. Both converged on the same real issue from different angles (undo-task safety
under a native fault), which was fixed and regression-tested during this review. Two
further findings were investigated and fixed (a narrower-than-claimed "compute once"
optimization, and thin NFD-normalization test coverage); one native test-coverage gap
was closed with new tests. All fixes were independently verified by full builds and test
runs, and one fix (the "compute once" caching) caught and corrected a real regression it
had itself introduced, verified before it reached this summary.
Contract/API Changes
IVwPattern2(new GUID) adds one method,ReplaceAllIn, to the native Views COMsurface. Verified additive: the generated MIDL header shows
IUnknown+ the complete,unmodified
IVwPatternvtable +ReplaceAllInappended last. TheVwPatterncoclasslists both interfaces;
QueryInterfacehandles both IIDs. No ABI break, no change tothe default coclass.
Findings
Critical - Must address before merge
None.
Important - Should address before merge
BulkEditBar.cs's outer bulk-edit loop (Doit(IEnumerable<int>, ProgressState)) could leave an unterminated undo task if a nativeReplaceAllInfault occurred mid-batch during a real apply, sinceBeginUndoTask/EndUndoTaskhad notry/finallyand the new bulk-replace path deliberately propagates exceptions rather than silently falling back. (fixed during review: wrapped the loop body intry/finallysoEndUndoTaskalways runs; addedDoit_OuterLoop_EndsUndoTaskWhenAnItemThrows, which fails without the fix and passes with it.)Minor - Consider
Native(fixed during review: addedTestVwPattern.h'sReplaceAllIntests didn't re-run the ORC, writing-system-restriction, case/diacritics, and NFD-equivalence scenarios that already existed asFindIn-only tests.testReplaceAllInReplaceCharPrecedingFinalORC_TE4727,testReplaceAllInRespectsMatchOldWritingSystem,testReplaceAllInWithCaseAndDiacriticsOptions,testReplaceAllInCanonicalEquivalence. All 27 VwPattern native tests pass, including the 4 new ones. One scenario —FindIn's "pattern spans an embedded, VC-omitted ORC" case — was deliberately not reproduced:ReplaceAllInalways searches through a rawTrivialTextSrc, which does not omit owned ORCs the way the VC-awareVwMappedTxtSrcused by that specificFindIntest does. This is an architectural difference in whatReplaceAllIn's contract covers (it operates on a plainITsString, not a VC-mapped text source), not a bug; a TE4727-style adjacent-ORC scenario was used instead to still exercise real ORC-preservation in the bulk path.)The "compute preview once" win only reached(fixed during review: addedReplaceWithMethod;BulkCopyMethodandTransduceMethodstill calledNewValuetwice per row inOkToChangeandTryGetNewValue.DoItMethod.NewValueCached, a per-call cache that lets anOkToChangeoverride share its computed value withTryGetNewValueinstead of recomputing. The cache is cleared at the end of everyTryGetNewValuecall so a later, separate call for the same row — e.g. preview, then apply — still recomputes, since the underlying design intentionally treats those as independent evaluations. AddedBulkCopy_ComputesSourceValueOnce. Self-caught regression: the first version of this cache did not clear between calls, which collapsedReplaceWithMethod's intentional preview-then-apply double-evaluation down to one, breakingFakeDoit_MatchesImmediateApplyAcrossPatternModes(7 failures). Caught by rerunning the full suite, fixed, and reverified at 75/75 passing.)NormalizeResult's NFD-skip check (IsNormalizedbefore callingget_NormalizedForm) was covered by only one Latin-diacritic test case (café).FakeDoit_NormalizesNonLatinScriptReplacementResultToNfd(Hangul syllable decomposition, a non-Latin script with a real multi-character canonical decomposition, unlike the single-diacritic Latin case) andFakeDoit_PreservesRichRunPropertiesWhenNormalizingNonLatinReplacementResult(a 1-character-to-3-character Hangul decomposition inside a styled, alternate-writing-system run, confirming run-property/offset-fixup survives a stronger decomposition than the existing café case). Both pass.)Required Validation / Evidence
./build.ps1(full native + managed) - 0 warnings, 0 errors../test.ps1forxWorksTests(ReplaceWithMethodPreviewTests,ReplaceAllInDecoratorCorrectnessTests,BulkEditBarTests,VwPatternReplacementTests) - 75/75 passed against the final combined build../test.ps1 -SkipManaged -TestProject TestViews(nativeVwPatternsuite, isolated viaTestViews.exe -v VwPattern) - 27/27 passed, including the 4 new cross-coverage tests../test.ps1 -SkipManaged -TestProject TestViews(full native suite, twice) - both runs reportTests [Ok-Fail-Error]: [309-0-0](all pass). Both runs then hang for ~5s during process teardown in an unrelated Uniscribe/Graphite rendering-engine subsystem (FindBreakPoint returned an error code), whichtest.ps1reports as a failure after killing the hung process. This subsystem is not touched by this branch's diff, the hang is fully reproducible independent of any change here, and it occurs strictly after all tests report passing. Treated as a pre-existing environmental flake in the native test harness, not a regression from this branch.Positive Observations
.idhsource)._allocabuffer to a persistentVector<OLECHAR>member so it survives multipleNextAcceptedMatchcalls across one bulk session.CheckedPatternPositionguards offset arithmetic againstintoverflow with an explicit failure instead of silent truncation.VwPatternReplacementTests.cscross-checks the real nativeReplaceAllInagainst a repeated-FindInoracle across regex, collation/locale, whole-word, writing-system/style/tag-run, RTL, and combining-mark edge cases — genuine end-to-end integration coverage, not native-only or managed-only.FakeDoit_PropagatesBulkReplacementFailureWithoutFallback), not an oversight — the review's finding was specifically about the interaction with the outer undo-task wrapper, now fixed.memcmpshortcut, an ordinal precheck, a collation-ignorable cache) are documented with why they were unsafe, inDocs/superpowers/plans/evidence/bulk-replacement-distillation-2026-08-13.md, rather than silently dropped.Interview Notes
BulkCopyMethod/TransduceMethoddouble-NewValuecall before deciding whether to fix or just document; investigation found only two call sites (OkToChangeoverrides in those two classes), no external callers ofOkToChangeoutside this file, and a freshDoItMethodinstance constructed per preview/apply phase (no cross-phase staleness risk) — low blast radius, so it was fixed rather than just documented.Suggested Review Focus
try/finallyfix and its regression test match the team's expectations for how a mid-batch native fault during a real (non-preview) bulk apply should behave.ReplaceAllIn-vs-VwMappedTxtSrcORC-omission architectural difference undocumented in code (noted here and in the PR) rather than adding a doc comment onIVwPattern2::ReplaceAllInitself.This change is