Perf/pipeline optimizations - #88
Closed
Mx-Iris wants to merge 33 commits into
Closed
Conversation
Every (supposedly debounced) keystroke in the runtime-object sidebar ran the full filter cascade synchronously on the main thread and reset `filterResult` on every row, whose didSet rebuilt the attributed title unconditionally — ~3.5 s of main-thread freeze per keystroke at 10k rows (20k title rebuilds on clear). Worse, the "500 ms debounce" never functioned: `.just(...).debounce(...)` flushes the pending element the moment the single-element source completes. - FilterEngine: extract a pure, thread-safe `match(_:haystacks:)` core; merge query/case/mode into `FilterContext: Equatable`; fix the inverted case-sensitivity branch in contains mode - SidebarRuntimeObjectCellViewModel: nil→nil guard on `filterResult`, equality guard on `filterContext`, cached `currentAndChildrenNames` with upward invalidation on child splices - SidebarRuntimeObjectFilterPipeline (new): snapshot (main) → verdicts (background, cooperative cancellation) → apply (main), replicating the legacy per-level ordering semantics (twin-tree parity test) - SidebarRuntimeObjectViewModel: `scheduleRefilter()` with generation tokens; broken debounce replaced by a working `delay(150 ms)` + flatMapLatest cancellation; empty queries keep the synchronous fast path - SidebarRuntimeObjectListViewModel: replace the uncancelled Task.detached Open Quickly search (two inflight searches raced on the same cell view models and a stale result could clobber a fresh one) with generation-guarded scheduling; stop cascading highlights into never-displayed child cells - SidebarRuntimeObjectViewController: the case-sensitivity button starts .on so the effective default stays case-insensitive now that the engine honors the flag Measured (debug, N = 10k): contains keystroke 3654 → 54–81 ms with 0 title rebuilds; clear 3763 → 17–20 ms; fuzzy narrow 4132 → 331–423 ms; seeded reload 7429 → 227–293 ms. The new regression suite pins the per-keystroke rebuild counts. Docs: Documentations/Plans/2026-08-04-sidebar-filter-pipeline-perf.md
ContentTextViewModel ran a single combineLatest(object, options, theme, transformer) → XPC fetch → main-thread NSAttributedString build. Every font-size/theme tweak paid a full XPC round-trip for an interface that does not depend on the theme, then rebuilt the whole attributed string on the main thread. This lands PR1 of the 2026-05-17 plan. - Fetch half: object / options / transformer (distinctUntilChanged) → engine; theme no longer participates. Render half: latest interface × latest theme → background-scheduler build → main-thread bind; flatMapLatest drops superseded builds on font-size click bursts - Move `catchAndReturn(nil)` inside the inner fetch sequence: on the outer chain it completed the whole pipeline on the first fetch error, permanently freezing the tab's content (regression test added) - Observable.tracking: never resolve @dependency inside the access closure — the re-arm hop runs on a bare main-queue dispatch, drops task-locals, and re-resolves against the ambient default context, silently swapping in a wrong instance and killing the chain. Both call sites (ResolvedThemeStream, the transformer stream) now capture the Settings instance at arm time; the contract is documented on the bridge - SemanticString builder returns an immutable copy (cross-thread handoff contract for the background-built string) - Core: GenerationOptions gains Equatable; RuntimeObjectInterface gains a public memberwise init (test stubs) - Signposts content.interfaceFetch / content.attributedStringBuild (category Content.TextPipeline) gate the follow-up PR2/PR3 decisions Tests: ContentTextPipeline suite — a theme-only change keeps the fetch count at 1 while re-rendering with the new font size, a failed fetch recovers on the next options change, and the render helper output is byte-equal to the direct builder invocation (PR2 restyle baseline). Docs: Documentations/Plans/2026-08-04-content-text-pipeline-pr1.md; the 2026-05-17 plan status now records PR0+PR1 as landed, PR2/PR3 gated on signpost measurements.
Every navigation step (link click, tab switch, back/forward) rebinds a fresh ContentTextViewModel, and each rebind re-fetched the interface over XPC — revisiting an object always paid full price, and a single link click actually fetched twice (resolution with bare default options, then display with the user's merged options under a different key). Add a per-document RuntimeInterfaceCache (LRU 16, keyed by object + merged generation options) and route every single-object fetch through it: the content pipeline's fetch half, both link-resolution flows (now using the same merged options, so resolution warms the entry the post-push display fetch hits — one round-trip per click), and MainViewModel's save/share paths. Concurrent lookups share one in-flight task; nil results and errors are never cached; engine swaps and dataChangePublisher events flush everything, with a generation token so a straggler fetch can never repopulate a flushed cache. Routing save/share through the merged options also fixes an existing inconsistency: exported text could differ from the displayed text because the transformer configuration never participated there. GenerationOptions and its members gain Hashable (additive, synthesized) to serve as cache keys. Regression suites cover hit/miss, coalescing, invalidation (including the real reloadData broadcast wiring), LRU eviction, and a navigation-revisit integration test.
swift-testing runs tests concurrently across suites while every DocumentState in the target shares RuntimeEngine.local. The real-engine flush test broadcasts .fullReload into whatever other suite happens to be mid-assertion, and the engine's first connect() replays one startup .fullReload from an unstructured Task at an arbitrary moment early in the run — both flakes only became visible once the test count grew. Adds withSharedLocalEngineLock (mutual exclusion for broadcasters and broadcast-sensitive tests) plus a one-shot startup barrier that waits out the engine's bring-up traffic, and wires the three existing sensitive tests through them.
Every image reload eagerly built a second full copy of the sidebar's cell view models for Open Quickly (icons, attributed titles, child trees) — ~250 ms of main-thread work per 10k rows in a debug build, paid even by sessions that never open the panel. The reload now stores only the sorted RuntimeObject array. Matching runs off-main against pure haystack strings (computed once per reload, byte-identical to the cell's own haystack so fuzzy highlight ranges still map — pinned by a parity test), and only matched rows materialize into cell view models, cached by row index so keystrokes reuse instances and DifferenceKit keeps stable row identities. The class drops `final` so tests can seed the real reload path, same as its superclass.
Root-sidebar keystrokes ran a recursive localizedCaseInsensitiveContains cascade over the whole image tree on the main thread, including the first-use recursive aggregate-name concatenation — thousands of nodes per keystroke on a dyld shared cache tree. Replaces the didSet cascade with a SidebarRootFilterPipeline mirroring the runtime-object pipeline's shape (snapshot on main, verdicts on the global executor, generation-guarded apply on main) while replicating the root tree's own legacy semantics: aggregate-contains matching, and a node whose own name matches shows its subtree unfiltered. Aggregate names now build inside the off-main verdict pass, so the cell's lazy aggregate property (and its main-thread first-use cost) is gone. Parity with the legacy semantics is pinned against an independent reference implementation.
Every itemDidExpand/itemDidCollapse notification walked all rows and wrote UserDefaults. An option-click "expand all" posts one notification per expandable item, turning the burst into O(rows squared) row visits plus a defaults write per item. The persist is now scheduled once per burst and flushed on the next main-queue turn, with the preconditions re-checked at flush time. A package-visible persist counter seams the coalescing for the regression test (the test target gains a RuntimeViewerUI dependency for it).
One landing doc for the three fixes (Open Quickly lazy materialization, root-sidebar off-main pipeline, outline autosave coalescing) plus the cross-suite test-isolation discovery, and marks the sidebar plan's follow-up items 1 and 4 as landed.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR focuses on eliminating main-thread stalls by moving several UI-critical pipelines (sidebar filtering, root sidebar filtering, Open Quickly search, content text rendering) off-main, and by introducing a per-document interface cache so navigation/save/share flows don’t repeatedly round-trip to the runtime engine.
Changes:
- Introduces off-main “snapshot → verdict → apply” filter pipelines for both the runtime-object sidebar and the root image tree, plus keystroke coalescing and cancellation/generation guards.
- Adds a per-document
RuntimeInterfaceCacheand routes single-object interface fetches (content, save/share, link resolution) through it using merged generation options. - Coalesces
StatefulOutlineViewexpansion autosave to avoid O(N²) bursts; adds comprehensive regression/perf test suites and supporting test infrastructure.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| RuntimeViewerUsingAppKit/RuntimeViewerUsingAppKit/Sidebar/RuntimeObject/SidebarRuntimeObjectViewController.swift | Sets case-insensitive toggle default to preserve prior effective behavior. |
| RuntimeViewerUsingAppKit/RuntimeViewerUsingAppKit/Main/MainViewModel.swift | Save/share now uses interface cache + merged options for consistency and cache hits. |
| RuntimeViewerPackages/Tests/RuntimeViewerApplicationTests/StatefulOutlineViewAutosaveTests.swift | Regression tests for coalesced expansion autosave behavior. |
| RuntimeViewerPackages/Tests/RuntimeViewerApplicationTests/SidebarRootFilterPipelineTests.swift | Root sidebar pipeline parity + end-to-end VM tests. |
| RuntimeViewerPackages/Tests/RuntimeViewerApplicationTests/SidebarFilterPerformanceBaselineTests.swift | Baseline/perf regression assertions for sidebar filter hot path. |
| RuntimeViewerPackages/Tests/RuntimeViewerApplicationTests/SharedLocalEngineTestLock.swift | Cross-suite lock + startup barrier for RuntimeEngine.local interference. |
| RuntimeViewerPackages/Tests/RuntimeViewerApplicationTests/RuntimeInterfaceCacheTests.swift | Contract tests for interface cache (LRU, invalidation, in-flight coalescing). |
| RuntimeViewerPackages/Tests/RuntimeViewerApplicationTests/OpenQuicklyLazyConstructionTests.swift | Tests for Open Quickly lazy row materialization + haystack parity. |
| RuntimeViewerPackages/Tests/RuntimeViewerApplicationTests/MockRouter.swift | Router test double to assert navigation side effects. |
| RuntimeViewerPackages/Tests/RuntimeViewerApplicationTests/ContentTextPipelineTests.swift | Content pipeline regression tests (no refetch on theme-only changes, error resilience, cache revisit). |
| RuntimeViewerPackages/Sources/RuntimeViewerUI/AppKit/StatefulOutlineView.swift | Coalesces expansion autosave persists; adds persist-count seam. |
| RuntimeViewerPackages/Sources/RuntimeViewerArchitectures/Observable+Tracking.swift | Documents dependency-resolution constraint inside tracking closures. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/ViewModel.swift | Adds currentMergedGenerationOptions for cache-key alignment and export consistency. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Theme/SemanticString+ThemeProfile.swift | Ensures immutable NSAttributedString escape for cross-thread rendering. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Theme/ResolvedThemeStream.swift | Resolves dependencies outside tracking closure to avoid context loss. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Sidebar/SidebarRuntimeObjectViewModel.swift | Replaces synchronous cascade with cancellable off-main filter pipeline apply. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Sidebar/SidebarRuntimeObjectListViewModel.swift | Open Quickly: lazy materialization, off-main matching, cancellation/generation guards. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Sidebar/SidebarRuntimeObjectFilterPipeline.swift | New runtime-object tree filter pipeline (snapshot/verdict/apply). |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Sidebar/SidebarRuntimeObjectCellViewModel.swift | Adds haystack caching, guarded title rebuilds, and pipeline apply entry point. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Sidebar/SidebarRootViewModel.swift | Root sidebar filtering moved off-main with cancellation + generation guard. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Sidebar/SidebarRootFilterPipeline.swift | New root tree filter pipeline replicating legacy semantics off-main. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Sidebar/SidebarRootCellViewModel.swift | Removes mutating filter cascade; adds pipeline apply entry point. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/FilterEngine.swift | Refactors into pure match + contextual filter; fixes case-sensitivity inversion. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/DocumentState.swift | Adds per-document interfaceCache. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Content/RuntimeInterfaceCache.swift | New LRU cache with invalidation + in-flight coalescing semantics. |
| RuntimeViewerPackages/Sources/RuntimeViewerApplication/Content/ContentTextViewModel.swift | Splits fetch/render halves, instruments with signposts, routes via cache/provider. |
| RuntimeViewerPackages/Package.swift | Adds RuntimeViewerUI dependency to application tests for package seam access. |
| RuntimeViewerCore/Sources/RuntimeViewerCore/Core/RuntimeSwiftSection.swift | Adds Hashable conformance needed for option-keyed caching. |
| RuntimeViewerCore/Sources/RuntimeViewerCore/Core/RuntimeObjCSection.swift | Adds Hashable conformance needed for option-keyed caching. |
| RuntimeViewerCore/Sources/RuntimeViewerCore/Common/RuntimeObjectInterface+GenerationOptions.swift | Makes GenerationOptions Equatable/Hashable for caching and distinctness. |
| RuntimeViewerCore/Sources/RuntimeViewerCore/Common/RuntimeObjectInterface.swift | Adds public init for easier construction in cache/pipeline tests. |
| Documentations/Plans/2026-08-04-sidebar-filter-pipeline-perf.md | Design/implementation record for sidebar filter pipeline perf refactor. |
| Documentations/Plans/2026-08-04-openquickly-root-outline-perf.md | Design/implementation record for Open Quickly + root filter + outline autosave. |
| Documentations/Plans/2026-08-04-navigation-interface-cache.md | Design/implementation record for navigation interface cache behavior/contract. |
| Documentations/Plans/2026-08-04-content-text-pipeline-pr1.md | Design/implementation record for content pipeline PR1 split. |
| Documentations/Plans/2026-05-17-content-text-attributedstring-optimization.md | Updates status to reflect implemented PR0/PR1 and measurement-gated PR2/PR3. |
| AGENTS.md | Codifies “single-object fetch via interface cache + merged options” rule. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
408
to
+410
| @MainActor | ||
| private func rebuildFilteredNodes() { | ||
| let scope = scope | ||
| func scheduleRefilter() { | ||
| currentFilterTask?.cancel() |
Comment on lines
77
to
83
| public var children: [SidebarRuntimeObjectCellViewModel] { | ||
| get { _filteredChildren } | ||
| set { | ||
| _children = newValue | ||
| _filteredChildren = newValue | ||
| invalidateNamesCacheUpwards() | ||
| } |
Comment on lines
+48
to
+50
| /// Generation guard for `currentOpenQuicklyFilterTask` — also bumped | ||
| /// when `nodesForOpenQuickly` is rebuilt, so a match computed against | ||
| /// a discarded node array is never applied. |
Every high-cardinality cell ViewModel carried five discrete @observed appearance properties (primaryIcon, secondaryIcon, tertiaryIcon, title, subtitle), and each @observed costs a BehaviorRelay wrapping a BehaviorSubject plus an NSRecursiveLock — roughly 450-500 bytes of Rx plumbing per property for values that only change on filter edits and specialization splices. With ~13k resident image-list rows and ~7k browse-path rows, the per-row multiplier made NSRecursiveLock the largest ObjC class in the process (125,225 instances after a full browse). Merge them into one @observed appearance struct per row, published atomically with an equality guard so identical refreshes emit nothing. RuntimeObjectCellDisplayable shrinks to a single appearanceDriver, cell views bind once and fan out to their outlets in apply(_:), and the remaining conformers (Inspector cells, the specialization type picker) compose their structs at init. Measured on the same five-image full-browse load (evolution proposal 0005, Implemented): NSRecursiveLock 125,225 -> 42,218, the UI/Rx heap cluster 46.7 -> 17.7 MiB, steady state 210 -> 196.7 MiB. Behavior is pinned by the existing filter-baseline emission counts plus new SidebarCellAppearanceTests (one event per transition, zero events for equal republish and display-neutral splices).
…ine flag FilterEngine's pre-2026-08 plain-contains branch had the flag inverted (isCaseInsensitive == true selected the case-SENSITIVE contains). When the engine was fixed to honor the flag, the AppKit sidebar flipped its toggle default in the same change, but the UIKit sidebar's hardcoded .just(false) was missed — shipping iOS a case-sensitive search (PR #88 review, finding 2; known-issue PR88.2). Flip the constant and pin the honest semantics at the engine with FilterEngineCaseSensitivityTests, so a future inversion fails loudly at the source instead of silently flipping whichever platform forgot to compensate. The engine-level suite stands in for a UIKit-side reproduction test: that target has no test bed, and the engine semantics are the root the regression grew from.
Splitting the content pipeline left trackActivity on the fetch half only. With a warm interface cache the fetch is near-instant, and theme / font-size changes skip it entirely, so every wait the user actually perceives fell in an untracked gap and the loading indicator never appeared (PR #88 review, finding 3; known-issue PR88.3). Track the render half's inner sequence too — the fetch's element reaches it before the fetch observable completes, so the indicator hands over without a false gap. Also hoist the ConcurrentDispatchQueueScheduler out of the flatMapLatest closure: the convenience initializer allocates a fresh DispatchQueue, so a burst of font-size clicks churned one queue per emission (finding 6; PR88.6). New test fontSizeChangeSurfacesLoadingIndicator fails against the fetch-only placement (verified red before this fix, green after).
…pshot scheduleRefilter() built the full snapshot forest before checking shouldFilter, so clearing the search (and the refilter right after every reload, when the per-cell haystack caches are cold) paid a bottom-up O(nodes) name build whose verdicts are, by definition, the identity (PR #88 review, finding 4; known-issue PR88.4). Serve the fast path with a new snapshot-free resetToUnfiltered that installs the same applyFilterOutcome the snapshot -> verdicts -> apply chain produces for an empty context — child-before-parent, identity child lists, no haystack reads. SidebarFilterFastPathTests pins the equivalence against the legacy chain on the same tree shape, and that clearing a real filter restores the identity state.
FilterEngine.filter ran match() — materializing the filterableString array and a verdict per item — one line before the empty-query guard discarded the result (PR #88 review, finding 5; known-issue PR88.5). Hoist the guard above the call; the context stamping loop stays first, pinned by FilterEngineCaseSensitivityTests.emptyQueryFilterResetsItems.
…ntract The filter-pipeline rework (cbb589c) removed the last read of the cell view model's appDefaults dependency; @dependency doesn't trigger unused warnings, so the property lingered as dead weight on a ~10k-instance class (PR #88 review, finding 11; PR88.11). SidebarRootFilterPipeline.verdicts(for:query:) silently clears the whole image tree for an empty query (localizedCaseInsensitiveContains("") is false for every haystack); the sole caller's fast path upholds the documented contract today, so assert it — a second call site is where it would break silently (finding 8; PR88.8).
Seven findings fixed this batch (each row carries its commit), one false positive retracted with the runtime evidence preserved — the RxCocoa-vs-RxAppKit control-property priming boundary is the part worth keeping — and seven backlog items with their pickup conditions. The KnownIssues index row lands on next alongside the other doc-index rows, since this branch predates the Documentations index.
Mx-Iris
added a commit
that referenced
this pull request
Aug 9, 2026
Coalescing the expansion persist onto the next main-queue turn left a window in which the tree could be replaced before the walk ran. The walk describes whatever tree is installed at flush time, and a rebuilt tree comes back fully collapsed — the root sidebar maps every `$nodes` emission through a fresh `SidebarRootCellViewModel` whose `Differentiable` conformance resolves `differenceIdentifier` to `self`, so every row is a new item — so the flush collected nothing and wrote an empty array over the user's saved state. `RuntimeEngine.reloadData` broadcasts `.fullReload` on every image load, and `restoreExpansionFromAutosave()` runs once per document, so the loss was both routine and permanent. Track a monotonic structure version, bumped by every entry point that can reshape the item tree, and sample it when the persist is scheduled; the flush runs only while the sample still matches. Expand/collapse notifications are delivered synchronously — `NotificationCenter` runs the block inline when the observer queue is the posting queue — so the sample always describes the tree the user acted on. The incremental mutators are hooked alongside `reloadData()` because a diffing adapter prefers them: RxAppKit only falls back to `reloadData()` when the changeset carries `elementUpdated` entries, which an all-new row set never does. The guard keys on the data changing, not on the walk coming back empty — collapsing every row is a legitimate way to persist an empty set, and the third new test pins that.
… rebuild
The image-tree rebuild used to install the new list and invalidate the
in-flight filter pass through two separate subscriptions:
`$nodes.bind(to: $filteredNodes)` ran synchronously, while the
cancellation and generation bump went through `subscribeOnNextMainActor`,
which expands to `Task { @mainactor in … }` and therefore only enqueued
them. A verdict continuation resuming in that window saw
`Task.isCancelled == false` and its captured generation unchanged, so it
passed both guards and applied cleanly — the old array is self-consistent
with its own snapshot — republishing the discarded cell tree over the
fresh one. Nothing reschedules a filter afterwards, so the image sidebar
kept showing the previous tree until the user typed again.
Merge both halves into one synchronous `installRebuiltNodes(_:)`. The
sibling `SidebarRuntimeObjectViewModel` already bumps its generation
synchronously inside `scheduleRefilter()`, so only the root pipeline had
this hole.
The tests sample the generation from a `$filteredNodes` observer rather
than after `accept` returns: `observe(on: MainScheduler.instance)` only
delivers synchronously while the scheduler is idle, so an
"assert right after accept" test passes alone and fails under concurrent
suites.
A link click asks the engine about a synthetic target built at the click site from the clicked token — it carries the currently displayed object's `imagePath`, and on the ObjC arm its `children` — and the engine answers with the defining section's authoritative `RuntimeObject`. That resolved object is what the push navigates to and what the destination `ContentTextViewModel` fetches under, but the entry was stored under the requested object. `RuntimeObject`'s `Hashable` folds in `imagePath` and `children`, so the display fetch was a guaranteed miss: two full generations per link click, plus a dead entry occupying one of the sixteen slots — the opposite of the one-round-trip design the link flow documents. The Swift arm rebuilds every field, so this hit same-image jumps too, not only cross-framework ones. Store the ready entry under `interface.object`.
The haystacks depend only on the object list, never on the query, but the apply task installed them behind the cancellation/generation guard — a pass superseded by the next keystroke threw its completed build away. Whenever the build outran the 150 ms debounce, continuous typing discarded one full build per query and the cache never populated. Install the build as soon as it completes, keyed to a new object-list version counter rather than the filter generation: the generation also moves on every keystroke, while the build is only invalid once a reload replaces the list it was built from (installing then would misalign every row index). The builder is injectable now so the regression test can gate the superseded pass's build and release it alone; releasing every gated build would let the current pass install the cache itself, which the old always-discard code also did, masking the regression.
…aystack Stamping a highlight on a freshly materialized cell triggers composedTitle(), whose cold currentAndChildrenNames rebuilt the whole subtree name string on the main actor — the byte-identical twin of the haystack the off-main matching pass had just computed for that same row (the two sides are byte-for-byte equal by the parity contract pinned in OpenQuicklyLazyConstructionTests). Hand the pass's haystack to the cell at materialization time via a seeding entry point on the cell view model. Seeding is a no-op once a value is cached, so it can never contradict a locally derived haystack.
.fuzzySearch keeps every haystack with a non-zero score, and a haystack is the object's name plus every descendant's, so a one- or two-character query matches essentially the whole image. The apply loop then built a cell view model — and, through rebuildChildren(), one per descendant, each with icon lookups and an attributed title — for every row in a single main-actor turn: the exact O(N) main-thread cost lazy materialization exists to remove, re-paid after every reload's first wide query, and retained in the row memo for the document's life. fuzzyMatch returns matches sorted by descending weight, so taking the prefix keeps the best-scoring rows; what the cap drops is the near-zero-score tail nobody scrolls to.
Records the cross-session re-verified adjudications for review findings F1-F15 as PR88R2.<N>: six fixed in-branch (with fix commits), the one-tick transformer skew and the unreachable applyNodes guard downgraded to no-fix with rationale, the specialization dead-entry claim refuted (any dataChangePublisher event flushes the whole interface cache), and six backlogged. Also adds the missing index rows for the 2026-08-09 and 2026-08-10 adjudication files.
Two retrospectives written while the PR #88 fixes landed but never committed; they were sitting untracked in the branch's worktree. The first covers the three Open Quickly performance fixes — the haystack seeding, the superseded-pass install, and the top-500 materialization cap. The second covers F9/F10/F11: where the test blockers were, how each fix was verified red-then-green, and why the batch was split the way it was. TaskReports/ is a new category on this branch. Documentations/README.md does not exist here — this branch forked before the index landed on main, which is also why Documentations/Evolution/ and Documentations/Evolutions/ still sit side by side. Registering these two files in the index therefore belongs with the rebase, alongside the documentation remediation already tracked as PR88.15 in KnownIssues/2026-08-09-pr88-review-findings.md.
…tched Filing the answer under `interface.object` is what makes the post-push display fetch hit, but the request key — the synthetic object built at the click site — was cleared and never written back. So every later click on the same type token missed and regenerated the whole interface with whatever detail flags the content pane carries, while the byte-identical answer sat one key over. Back/Forward and "Open in New Tab" over one token repeated that forever. A request key now learns where its answer was filed and follows that redirect on the next lookup. The redirect table is flushed with the entries, since a reload can move a type to a different image; a redirect that outlives its target's eviction costs one dictionary lookup and then misses exactly as it would have anyway. Clearing the in-flight entry is now conditional on still owning it. Two fetches with different request keys can converge on one storage key — a link click resolving a synthetic object into O while another tab fetches O directly — and the unconditional delete destroyed whichever entry the other had legitimately installed, stranding its key in `readyKeysByRecency` as a phantom that permanently consumed an LRU slot. That interleaving was previously judged unreachable; the redirect above makes request/storage key aliases routine, so it is closed here rather than left to become reachable. Both paths get a regression test that fails before this change.
…tack `composedTitle()` searched the row's entire subtree haystack for its own displayName to find where the highlight ranges apply. That name is the haystack's prefix by construction — `currentAndChildrenNames` builds "displayName child1 child2 …" and `haystack(for:)` matches it byte for byte — so the search could only ever return offset 0. `ranges(of:)` collects every occurrence, so it did not even stop at the first hit: one full scan of a subtree string per row, per keystroke. In fuzzy-search mode that runs on essentially every row, because fuzzy matching keeps every haystack with a non-zero score and the `filterResult` didSet only short-circuits the nil -> nil transition. The 500-row cap added for Open Quickly has no counterpart on the sidebar tree. Offsets stay in Characters to match `integerRange(from:)` (`distance(from:to:)`), so the ranges compared below are unchanged.
…n one turn Three fixes on the same path, all of them the unfinished half of an earlier one. Haystack builds are now shared. `cachedHaystacks` was sampled at schedule time and never re-read, so every keystroke landing during the first build started its own full O(N) build of the identical array. Cancelling the superseded pass freed nothing — `defaultHaystackBuilder` has no cancellation points — so the builds simply ran concurrently. A pass now joins the build already running for the same object list, the shape `RuntimeInterfaceCache` already uses. 523d98d stopped a superseded pass from *discarding* its completed build; this stops the redundant build from starting. Clearing stale highlights now touches only the previous pass's matches instead of the whole materialized-cell map. That map is deliberately kept warm across searches (see OpenQuicklyLazyConstructionTests), so it accumulates every row any query has surfaced, and sweeping it whole made per-keystroke main-actor cost grow with session length. Reload invalidation moves into `didInstallReloadedNodes()`, a new base hook called inside the same synchronous block that installs `nodes`. Doing it in a later `MainActor.run` left a window — `reloadData()` suspends at every one of its `MainActor.run` blocks — where an in-flight pass could resume, find its generation token still current, and publish rows built from the pre-reload list. This is the Open Quickly sibling of the root-sidebar fix in 920c2aa. It carries no regression test: hitting the window requires a pass to resume *and* finish inside one main-actor hop, and `matchOffMain` always suspends, which hands the actor to the invalidation block. The rationale is recorded in the adjudication file. The superseded-build test now pins both contracts — one build, and its result installed — instead of asserting the redundant build it used to require.
Records the cross-session re-verified adjudications for the third max-level review of PR #88 as `PR88R3.<N>`. No finding was a regression this round — every one is an optimization that covered only half its ground, which is what the first two passes cannot say. Five fixed in-branch (with fix commits). Two merges the review proposed were overturned on re-verification and are registered independently: the root pipeline's cancellation check guards a forest of exactly two nodes, so every superseded pass runs to completion, and the remedy recorded under PR88R2.15 does not address it; the object pipeline's snapshot descending into scope-pruned subtrees is an overhead issue, not the staleness issue PR88R2.7 tracks. PR88.9 gains a note: the `dataStructureVersion` mismatch is a second silent-discard path, introduced by the PR88R2.1 fix and therefore absent when PR88.9 was written — fixing only the `filteringState` half would leave the bug in place. Five false positive / no-fix, including two the review got backwards. The removed outer `catchAndReturn` is strictly an improvement: the old placement completed the whole pipeline on first error. The phantom-LRU-key claim is reclassified from unreachable to reachable-but-harmless, with the interleaving that reaches it written out — and with the coupling neither earlier pass recorded: fixing the cache-key finding makes that interleaving routine, so "unreachable" verdicts now carry what would make them reachable.
`reloadData()`'s `.notLoaded` early return and `scheduleReload()`'s `.loadError` catch both end a reload as definitively as the branch that installs nodes, but neither ran the subclass invalidation hook. The override this replaced ran after `super.reloadData()` returned, and `.notLoaded` returns rather than throws, so both outcomes used to be covered. Without it an Open Quickly pass scheduled before the reload keeps its generation token valid and publishes rows for an image the engine no longer reports as loaded, the search string is never cleared, and the whole Open Quickly index stays resident for the document's life. Rename the hook to `invalidateNodeDerivedState()`: it no longer runs only where nodes were installed, and the old name would tell the next reader the opposite. Cancellation stays uncovered on purpose — the reload that superseded this one runs the invalidation itself.
…s haystacks A pass task's body is enqueued while `reloadData()`'s final `MainActor.run` may already be queued ahead of it, so the reload can land between scheduling and the body's first line and leave the captured object-list version describing a list that no longer exists. The three checks inside `openQuicklyHaystacks(forObjectListVersion:runtimeObjects:)` all key on that version, so all three miss: the superseded pass starts a full O(N) build over the discarded list, parks it in the shared in-flight slot where a later pass can no longer join the live build, and the version guard at the end then also skips clearing the registration — pinning the dead object array and the finished haystacks until the next reload. Moving the existing generation check ahead of the haystack call closes all of that; cancellation alone would not, since a cancelled task still runs its body from the start. Not covered by a test: reaching the window needs a main-actor job interposed between the task's creation and its body, which no seam exposes. Recorded in the pass-4 review findings.
The learned redirect is consulted before the request key's own entry and was only ever written, never erased. Once a request starts answering to itself — background indexing brings the defining image in, so the engine stops forwarding — the fresh interface lands under the request key while the old redirect still points one key over, and the lookup walks past the right answer for the rest of the document's life: every later request refetches, or is served the object the token used to resolve to. The field's own doc comment argued a stale redirect is harmless because "the following store overwrites it". In the one case that matters the store takes the other branch and does not overwrite it; correct the comment along with the code.
`Key` folded in the whole `RuntimeObject`, so `children` (recursive), `displayName` and `properties` all participated in cache identity. Two consequences: every lookup hashed a whole subtree, and a link click's synthetic target — which can only carry the *displaying* object's `children` and a `displayName` assembled from the printed token — could never match the authoritative object it resolved to, which is what the redirect table exists to paper over. `RuntimeObjectKey` is `(imagePath, name, kind)`, the identity `RuntimeSwiftSection.interfaceByObject` already uses for the very interfaces this cache stores, so a finer key here can only produce misses the engine itself does not have. Generic and specialized Swift types stay distinct because `name` is the mangled name. Same-image links now resolve to their own key and record no redirect at all, leaving only cross-image links in the table — which also removes the unbounded growth of a `[Key: Key]` map whose entries each pinned two recursive `RuntimeObject` graphs while the interfaces they named were capped at sixteen. Keeping `init(object:options:)` leaves both construction sites unchanged.
… reader Seeding existed so that stamping `filterResult` on a freshly materialized row would not make `composedTitle()` rebuild the whole subtree name string on the main actor. `composedTitle()` stopped reading `currentAndChildrenNames` when the highlight range became the constant `0..<displayName.count`, and nothing else reads it on a `forOpenQuickly: true` cell: the filter pipeline only walks the `forOpenQuickly: false` tree, and `filterableString` is reached through `FilterEngine.filter`, which returns at its empty-context guard for the one call an Open Quickly cell makes during `rebuildChildren()`. So the seeded string was stored and never read, behind a doc comment calling byte-for-byte parity a hard contract. Remove the seam, the `haystack:` parameter that fed it, and the two tests that covered it. Byte parity is no longer load-bearing — no range crosses from one builder to the other — but the *prefix* is: `composedTitle()` asserts the name starts the haystack rather than deriving the offset, and nothing pinned that. Assert it in the parity test for both builders, and rewrite the contract comment to say what actually depends on it.
Records `PR88R4.1`–`PR88R4.5` against the commits that fixed them, the `PR88R4.2` test gap and why no production seam was added for it, and the two findings adjudicated away on adversarial re-review: the `ResolvedThemeStream` `.forever` claim (pre-existing on main, and the render ordering prevents the consequence) and the Open Quickly 500-row cap (a re-report of `PR88R2.11`'s own fix). Also corrects two documents this pass found contradicting the code: - AGENTS.md §9 named `SidebarRuntimeObjectCellViewModel` as a type that "must stay eager" while this branch materializes exactly that type lazily. The Open Quickly scheme satisfies the rule's *reasoning* — a warm cache keeps instance identity across keystrokes — and violates only its wording, so the section now separates stateless `DifferentiableBox` laziness from lazy materialization behind a cache. - Evolution 0005's non-goals promised not to touch `SpecializationTypePickerCellViewModel` or the low-cardinality cell view models, and the landing record changed both while calling the change list consistent with the proposal. Proposals are decision snapshots, so the body stays as written and a "与提案的差异" section records it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.