Stabilization: pre-PR audit fixes, AppKit-first document tabs, transcription off-main#7
Open
m-szymanska wants to merge 25 commits into
Open
Stabilization: pre-PR audit fixes, AppKit-first document tabs, transcription off-main#7m-szymanska wants to merge 25 commits into
m-szymanska wants to merge 25 commits into
Conversation
A throwing engine.stopRecording() left isRecording=true and the cadence commit loop running, wedging the UI in Recording state. Defer the state reset so every exit path cleans up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…arget windowWillClose leaked the local keyDown monitor. The Shift+Return hotkey now only sends to the editor; dispatching an autonomous agent requires an explicit click on Send by design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…spatch Replace the hardcoded /Users/maciejgad vibecrafted launcher path with env-var + home-relative resolution (PENSIEVE_VIBECRAFTED_PATH override, LocalizedError when absent). The agent workspace root default is now the open workspace folder, falling back to the user home. Concurrent sends are rejected while a dispatch is in flight. The report-path fallback regex no longer matches URL substrings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WKPDFConfiguration.rect was pinned to the 794x1123 viewport, cropping everything below the first page. Leave rect unset so createPDF captures the entire content bounds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NSLog(message) with a dynamic string interprets % sequences as format
specifiers; switch to NSLog("%@", message). Log the file-protection
downgrade fallback in the cache/metadata stores and add visibility to
the cold-start/hot-reopen cache fallback catches that swallowed errors
with no trace. No control-flow changes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…options Aligns the simulated manifest corruption with the store's protected write options, resolving the semgrep swift-data-protection finding on the diff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The app binary links libqube_ffi.dylib unconditionally, so a bundle without the embedded dylib aborts in dyld at launch. Failing the build beats shipping a non-launching app with a warning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cargo bakes the builder's absolute checkout path into the dylib id, so binaries linked against it fail dlopen on every other machine -- CI included; swift test could never pass on a runner. Repoint the vendored binary to @rpath (Package.swift already adds the matching -rpath) and make build-ffi.sh apply the same fix on every future sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The math pipeline styled TeX but never rendered it: the bootstrap calls window.katex which was never bundled. Vendor KaTeX 0.17.0 via the new pinned scripts/vendor-katex.sh (runtime + stylesheet with woff2 fonts embedded as data: URIs, fully self-contained for loadHTMLString) and embed both into the composed HTML before the math bootstrap, mirroring the mermaid pattern -- including the incremental updateLoadedPage path, so a document gaining math mid-edit renders without a full reload. The ~640KB payload is embedded only for documents that contain math and the vendored assets are cached after the first disk read. Inline-embedding sanitization is now case-insensitive (</SCRIPT> variants) and shared by all script/style payloads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…atch The vendored qube-ffi bridge has no completion FFI symbol -- VistaEngine.complete() is a hand-added stub returning "" -- so wiring the real engine as the default would load the LLM on first keystroke only to render nothing. The default controller now has no engine and surfaces a clear lastError; swap the factory to VistaEngine once vista-kernel ships completion support. The model-init failure latch is now engine-global (armed even when the failing request was superseded by typing) with a single-flight guard so overlapping debounced requests cannot stack concurrent blocking initModel calls on the cooperative pool. cancel() still re-opens the deliberate retry path. MockVistaAutocompleteEngine moved out of production sources into the test target, fatalError stubs replaced, and the latch semantics are covered by four new tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Clicking a sidebar item opened the document via SwiftUI openWindow, which presents a fresh standalone window immediately; the registry could only addTabbedWindow it after the async document load reported a documentID, so the window flashed at full size for the load beat and then visibly merged into the original window. Two-part fix: the window accessor falls back to the scene's initialDocument id so the first attach already carries the document identity (no transient launcher registration, merge happens in the window's first main-queue turn), and a window whose document has a pending merge target is born with alphaValue 0 — the registry reveals it strictly after addTabbedWindow, so it first paints as a tab. A modal-deferred attach reveals immediately rather than leaving a window invisible for the modal's lifetime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… explicit A plain sidebar/search click routed through openWindow(value:), spawning a full cold SwiftUI scene per document (~0.5-1.2s of fresh AppState + controller). The half-built window was user-visible for that beat in some compositor layer no matter where we hid it -- 12 commits since a152d00 plus five suppression layers today all fought the same gap. Maciej's goal for a152d00 (native multi-window: 3 windows or 1, tabs, user-driven merges) does not require spawning a scene per click, so the default gesture now selects the document in the current window, exactly like the pre-tab routing. Native multi-window stays fully available: context-menu Open in New Window routes through the registry with today's hardening (in-flight coalescing so re-clicks cannot duplicate windows or clobber the merge target, pre-frame-commit presentation suppression, merge-then-content-ready reveal handshake with a 3s failsafe), plus system tabbing and Window-menu merges. External opens (Finder, recents) also load in-window; they previously took a system-spawned scene that bypassed the registry entirely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ocuments With in-window routing a window swaps its displayed document constantly, but windowsByDocumentID kept every document the window had EVER shown. Open in New Window for any previously-viewed document then hit the existing-window path and silently activated the current window instead of spawning one. A window hosts exactly one document: attach now drops the window's other mappings. The context menu also hides Open in New Window for the currently selected document, where one-window-per- document makes the gesture a designed no-op. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PENSIEVE_TRACE=1 enables NSLog tracing of window lifecycle notifications, open/attach/merge registry decisions, and document open routing. Off by default; a disabled call site costs one cached bool check. Replaces the throwaway instrumentation used while diagnosing the document-window flash so the next runtime investigation starts with signal instead of print-debugging from scratch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Document windows are now built directly in AppKit (DocumentWindowFactory hosting the same per-window SwiftUI root, toolbar/title bridged via sceneBridgingOptions) and attached to the native tab group BEFORE first presentation -- the standalone-window flash of the openWindow(value:) scene path is impossible by construction, so all five suppression layers (accessor alpha, didBecomeKey hide, pre-CA-commit runloop sweep, merge-target bookkeeping, content-ready reveal handshake) are deleted as dead code. Default click routing returns to tab per document; a document already open anywhere activates its window. The native tab bar plus button opens an untitled tab in the same group instead of a detached window, and is deferred (not dropped) during modal run loops. Closing a factory tab tears down its hosting view to break the window -> SwiftUI view graph -> window retain cycle, unregisters the document so it re-opens fresh (no zombie resurrection; belt: a torn- down window is never activated from a stale mapping). A launcher window that is a member of a document tab group is recognized as the plus-button result on SwiftUI-origin tabs and is no longer reaped. External opens route to their own tab without contaminating the originating window's working set. Known v1 gap, deliberate: factory document tabs do not participate in window state restoration across relaunches (the legacy WindowGroup restoration path still works for old scenes). Toolbar bridging needs macOS 14+; the test runner already targets macOS 14. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Hang report from the field: tapping Start in the tafla called VistaEngine.initModel() inline on the main actor, and the Rust side dequantizes the whisper weights there (dequantize_q8) -- the entire UI froze for the duration of the model load (1.77s+ sampled). Model init and capture start now run on a detached task; the panel shows an explicit preparing state (spinner on the Start button, 'Preparing' status) and init failures surface through lastError instead of a throwing synchronous call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Field repro (traced): closing a tabbed document window synchronously tore down its hosting view mid tab-group reshuffle, leaving a half-dead ghost on screen (visible, unmovable, invalid for accessibility), and the closed window's SwiftUI accessor fired one final main-queue pass that re-registered the closed window -- closed tabs then 'jumped back' and the same document showed up in two windows. Three changes: the close() teardown is deferred one runloop turn so AppKit finishes the tab-group dance first; attach() rejects windows the registry has seen close (late accessor passes can no longer resurrect them); and the empty-launcher sweep only spares a group-member launcher while it is the KEY window (the fresh plus-button tab) instead of every launcher that ever joined a group, which had let empty 'Pensieve' tabs accumulate during tab churn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Field traces showed 10-15 redundant registry.attach calls per interaction: the window accessor dispatched one on EVERY SwiftUI render pass of every window, and each attach queued its own launcher-sweep timer. The accessor coordinator now remembers what was last attached and skips no-op passes, and the close-sweep coalesces to a single pending timer that reads the latest spared window at fire time. Pure main-thread churn reduction for tab switching; no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces native tab-based document window routing via a new DocumentWindowFactory and DocumentWindowRegistry to eliminate standalone window flashes, vendors and integrates KaTeX for local math rendering in previews, and offloads speech model initialization to background tasks to prevent UI hangs. Feedback on these changes highlights opportunities to prevent memory leaks and improve robustness, specifically by purging the closedWindows dictionary to avoid memory bloat, using viewDidMoveToWindow() instead of DispatchQueue.main.async for window attachment, and capturing self weakly in the global event monitor to avoid a retain cycle.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5863aa343
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The closed-window tombstone map only needs an entry while the NSWindow object is still alive (attach() compares identity against the live instance). Entries whose weak reference has gone nil are dead weight that accumulated forever; drop them in purgeClosedLauncherWindows() alongside the other weak-window maps. Addresses gemini-code-assist review on PR #7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The accessor relied on a single one-runloop-turn dispatch to find the view's window; if the window arrived later than that turn and no further SwiftUI update fired, the attach was silently dropped. Use an NSView subclass whose viewDidMoveToWindow() reinvokes attachIfNeeded, giving a guaranteed AppKit signal whenever the window slot changes. The deferred main-queue hop and the coordinator's coalescing stay as-is, so registry side effects remain out of the layout pass and deduplicated. Addresses gemini-code-assist review on PR #7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Task.detached does not inherit cancellation, so cancelling startRecordingTask in deinit left the detached model load running and let it start the microphone after the owning service/window was gone. Bridge cancellation into the detached task via withTaskCancellationHandler, check cancellation right before engine.startRecording, and stop the engine if capture managed to start while the owner went away mid-await. Regression test: teardown during a blocked model load must never reach engine.startRecording (verified failing against the previous code). Addresses chatgpt-codex-connector P2 review on PR #7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88628eb8f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The registry route in openFile() bypassed registerOpenFile, the only gate rejecting non-Markdown/text extensions, so opening an unsupported file from Finder/Cmd-O while a document was showing created an empty tab whose load was refused only afterwards. Reject unsupported URLs before routing and surface the shared validation error instead; the message now lives in one place (WorkspaceScanner.unsupportedOpenMessage). Regression test: an unsupported file never reaches requestOpenDocumentWindow (verified failing against the previous code). Addresses chatgpt-codex-connector P2 review on PR #7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbd7a02218
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The accessor fell back to the scene's initialDocument indefinitely, so a factory/restored window whose URL failed to load (deleted or unreadable recent) kept advertising the document to the registry; the empty error window stayed mapped to the URL and subsequent opens only reactivated that dead tab. The fallback now ends when the synchronous initial load resolves: success reports the real selection, failure reports nil so the window re-registers as a launcher and the document mapping is released. Decision extracted to accessorDocumentID() and unit-tested. Addresses chatgpt-codex-connector P2 review on PR #7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
In the Preparing state both the start control (disabled, spinner) and the stop button (gated on isRecording) were dead, so a slow or hung model load left the panel with no way out — and hiding the panel does not deinit the window-controller-owned service. The stop button is now enabled during preparation and routes to cancelPreparation(), which cancels the bridged prepare task (the existing rollback stops the engine if capture already began) and leaves the Preparing state immediately. Regression test: cancel during a blocked model load never starts the microphone and surfaces no error. Addresses chatgpt-codex-connector P2 review on PR #7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 615ec454fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A request arriving while another debounced request was still running initModel() bailed out of the single-flight guard, and the original request's completion was then discarded as stale by the requestID check — so the current prefix never got a suggestion until another keystroke. The single-flight bool is now a shared Task: every overlapping request awaits the SAME init and proceeds to complete() the moment the model is ready. The engine-global failure latch keeps its semantics (armed by the init task even when all awaiting requests were superseded; UI writes stay gated on request currency). Regression test: a keystroke landing mid-init joins the shared init and receives its suggestion after init completes, with init still running exactly once (verified failing against the previous code). Addresses chatgpt-codex-connector P2 review on PR #7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Full-day stabilization pass on top of
fix/broken-native-toolbar-and-others(18 commits). Three blocks:1. Pre-PR audit follow-ups (findings-max audit of PR #6)
PENSIEVE_VIBECRAFTED_PATHenv override + home-relative default,LocalizedErrorwhen absent), workspace root falls back to the open folder, in-flight guard against double dispatch, report-path regex no longer matches URL substrings; tafla Shift+Return sends to editor only (agent dispatch is click-only by design)scripts/vendor-katex.sh, fonts embedded as data URIs) — math actually renders now, also in exports; assets cached after first read; inline-embed sanitization case-insensitiveVistaEngine.complete()is a hand-added stub returning""), so the default controller now has no engine and surfaces a clear error instead of loading the LLM to render nothing; model-init failure latch is engine-global with single-flight guard; mock moved to the test targetNSLogformat-string fixes, file-protection fallback logged, silent catches surfacedlibqube_ffi.dylibrepointed to@rpath(without thisswift testfails on every machine except the builder's — CI included);build-ffi.shapplies it on every sync;build-release.shhard-fails when the dylib is missing (the app links it unconditionally)2. Window/tab architecture rebuilt (tab per document, AppKit-first)
The
openWindow(value:)scene-per-document path presented a half-built window for its 0.5–1.2 s cold start; five suppression layers could not fully hide it, and re-clicks during materialization clobbered merge targets (duplicate windows). Document windows are now built directly in AppKit (DocumentWindowFactory) and attached to the native tab group before first presentation — the flash is impossible by construction, all suppression machinery deleted.@State→ window retain cycle broken)PENSIEVE_TRACE=1) for window/document flow3. Transcription
Known gaps (deliberate, follow-up rounds)
NSHostingView.sceneBridgingOptions)Test plan
make testgreen at every commit (336 tests at HEAD, +30 vs base: registry merge ordering, close/teardown, coalescing, zombie guards, autocomplete latch, transcription preparing state)make lintclean on touched files🤖 Generated with Claude Code