Repository navigation
Background-resilient model downloads, imports, and summaries - #12
Merged
Merged
Conversation
Re-summarizing during an active import (or any two heavy MLX/ASR jobs overlapping) could exhaust device memory and crash the app. A FIFO gate now makes summarize/import/retranscribe/diarize-retry mutually exclusive, live recording waits for it to drain before loading its own models, and summarize refuses to start mid-recording. Also lands the Bonsai 8B (ternary 2-bit) summary model behind a Settings toggle, off by default — see docs/superpowers/plans/2026-06-20-meaningful-summaries.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Lets the UI language follow a manual pick (system/English/Simplified Chinese) instead of always tracking the device locale, so summary language and settings copy stay in sync with the user's choice.
Background transfers now run concurrently and persist URLSession resume data on cancellation or failure, so the next attempt continues the transfer instead of restarting it. Also fixes revision strings with slashes not being split into separate URL path components.
SerialGate.acquire() and the offline audio decode loop now check Task.checkCancellation() at their yield points, so a cancelled caller unwinds immediately instead of running to completion or leaking a held slot.
The DEBUG-only diagnostic logged note headlines and transcript-derived counts; remove it now that the map-extraction-on-garbled-input question is settled.
Recording chip taps were animating implicitly, causing a visible flash; disable the transaction animation on that row.
Tests read ModelCatalog.all directly, so a stray "model.bonsaiEnabled" default from another test run could change the expected lineup; scope each test to its own UserDefaults suite instead.
A killed or backgrounded file import currently loses all progress and the placeholder record itself. ImportCheckpoint stores the import options and per-segment transcript text needed to resume instead of restarting from zero — an all-optional nested struct, so existing persisted records keep decoding unchanged.
VAD segmentation is deterministic for the same audio, so a resumed import can match a freshly-discovered segment's time range against the checkpoint and reuse its text instead of re-decoding — and reports each newly-decoded segment via onSegmentComplete so a caller can checkpoint it immediately. Threaded through OfflineTranscriber and both file transcribers, same as the existing onProgress plumbing.
The durable audio copy used to happen last, after transcription, diarization, and translation all succeeded — so a kill mid-decode left nothing to resume from, not even the audio. Copy it first instead, via a new onAudioReady callback, and add a resumeImport entry point that reads straight from that durable copy (never the original picker URL, which may not be valid after a relaunch). The diarization/translation tail is now shared between a fresh import and a resumed one.
loadIfNeeded() and mergedLoadedSessions() treated every interrupted import as an unrecoverable tombstone and deleted it on relaunch. Now only imports without a checkpoint and durable audio (killed before either existed — genuinely nothing to resume) get swept; the rest survive so SummaryJobCenter can pick them back up.
startImport now persists the checkpoint as the import progresses (audio-ready, then each decoded segment), instead of only writing a finished record at the very end. resumeUnfinishedImports() restarts any session still marked importing from its checkpoint — called on cold launch and after a recording that preempted an import ends. Also fixes yieldToRecording() permanently cancelling an import when a new recording preempts it: it now marks the activity paused before cancelling, so the checkpoint survives and resumeAfterRecording() re-enqueues it instead of the placeholder being deleted.
Stock mlx throws std::runtime_error from a Metal completion handler when iOS aborts a command buffer at background entry — uncatchable, kills the process (the re-summarize background freeze). The fork (DakePeng/mlx-swift branch loqi/0.31.4-poison-events) is upstream 0.31.4 plus tseylerd's backport of ml-explore/mlx#3523: buffer errors poison the stream and surface at safe, catchable points, and poisoned events are force-signaled so the eval lock can never wedge. Drop the fork once mlx-swift vendors mlx >= a025496c. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Scene-exit hardening around the forked mlx: generation gates every prefill window and token boundary, loads run on the CPU stream, and a background abort now surfaces as MLXError — matched by content (not just the scene flag, which misses scenePhase delivery lag on both edges) and mapped to the abandon/suspend path so jobs restart from their chunk-note checkpoints instead of crashing or laundering the abort into fake notes. Resume correctness: resumes skip the hygiene pass when a usable map checkpoint exists (re-rolling nondeterministic hotword restores used to wipe the checkpoint and remap everything), the background persistence flush holds a BackgroundTaskGrace so a jetsam can't lose the last checkpoint, and pendingSummary markers restart interrupted summaries at launch. Redo semantics: a manual re-summarize with unchanged style+length clears map coverage and regenerates the notes — the cached-notes short-circuit used to reduce-only re-render the same notes, making the button appear dead. Style/length changes keep the cheap reduce-only path. Job-center diagnostics throughout (suspend/resume/checkpoint/rejection logging) so the next background bug is a one-log diagnosis. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Qwen3.5-2B drifts off the TSV record schema in reproducible ways — device diagnostics drove each countermeasure: - Per-line adaptive field splitting (tabs, 2+-space runs, single-space last resort) instead of one whole-response strategy; mixed-separator responses used to reject every line. - One-shot example in the prompt (six lines, per note language) ends spec-block parroting; a three-line example anchored output cardinality, so it demonstrates more record kinds. Verbatim example echoes are rejected by their distinctive content strings. - Response prefix "T\t<chunk>\t" appended after the assistant header so the first record line is well-formed by construction. - Any cNNN chunk id is accepted (only one chunk exists per call; the model borrows the example's c000 onto real records) — grounding stays enforced by the source-id check. - Salvage pass when strict parsing accepts nothing: compressed-schema near-misses (T without time range, uncited P-family, A without owner/ deadline) and id-led lines whose citations validate against the chunk's real source ids. Strict output always wins; validly-shaped lines rejected for cause stay rejected. - m-id citation tokens scrubbed from all content fields at parse time and at the reduce/render boundaries (covers previously persisted notes). - Resume progress counts cached chunks as completed steps so the UI never restarts at 0/remaining after backgrounding. - Privacy-safe parse diagnostics (rejection tallies, no content) log on every empty parse and salvage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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: 1ab1f5ddf6
ℹ️ 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".
- Live chunk notes now pass the same "T\t<chunk>\t" response prefix as batch summarize — the live path used the identical prompt and parser but had no first-line anchor, leaving it exposed to the 2B schema drift the batch path was hardened against. - isBackgroundGPUAbort matches two independent phrasings of the background-permission error (IOGPU constant + Metal description) so a wording change in one layer can't silently unmap aborts from the suspend path; TODO added to surface a typed code through the fork. - hasUsableMapCheckpoint now derives from SummaryEngine.uncoveredEntries instead of re-implementing its coverage semantics — the two can no longer diverge, and an all-stub checkpoint correctly counts as unusable (hygiene runs before the inevitable full remap). - Parser cleanups: tag→kind mapping extracted to recordKind(for:) (was copy-pasted in three places), and action records scrub the task field once instead of twice per record. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- resumeUnfinishedImports guards !isBackgrounded itself (stop-recording from the lock screen no longer restarts Metal-unsafe ASR in the background) and skips sessions with a held activity badge. - finishJob re-sweeps unfinished imports after teardown, so an import whose cancellation unwind outlived the foreground resume sweep is picked up when its task actually exits instead of stranding until the next cold launch. - cancel() tears down a held/unwound import's placeholder + checkpoint (mirroring the task's own user-cancel path) so "Cancel processing" on a paused import can't be resurrected by the next resume sweep. - A failed import drops its checkpoint: resume sweeps skip it and the checkpoint-less placeholder is swept at next launch — without this the finishJob re-sweep would loop a deterministic failure forever. - PendingSummary persists the user's download consent so a summary killed mid-download keeps downloading on restart instead of failing with "model not downloaded". - Session deletion kicks an immediate persistence flush so a force-quit right after deleting can't resurrect the record at next launch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36d2af4839
ℹ️ 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".
- Background launches (iOS relaunching for background URLSession events) seed scene state from UIApplication.applicationState before the startup sweeps, and resumeUnfinishedSummaries guards !isBackgrounded like the imports sweep — no Metal work from a background launch. - FileImportEngine rethrows CancellationError from the diarization catch instead of swallowing it as a label failure, so a background/ yield preempt actually stops the import at its checkpoint. - Auto ASR fallback split from the reader language: fallbackLanguage uses the new AppLanguage.speechPreferred (system language list only), so a zh UI + English speech in Auto mode no longer builds a zh recognizer; summaries keep the reader-language override. - Memory warnings full-unload the LLM during import-only jobs (new usesLLM predicate) instead of shed-only: imports never touch the model, so resident weights were pure jetsam bait. - A preempt before the durable audio + checkpoint landed deletes the placeholder instead of holding an unresumable "importing" row until the next relaunch sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review again |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
2 of 8 tasks
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.
What this branch does
Makes every long-running on-device job — model downloads, file imports, re-transcribes, and map/reduce summaries — survive backgrounding, memory pressure, and process kills, and hardens the summary map phase against small-model format drift. 35 commits, three arcs:
1. Resumable background work (downloads, imports)
URLSessionbackground sessions and resume after cancellation.SessionRecord.ImportCheckpoint), copy audio durably first, and resume from the checkpoint after a kill or backgrounding instead of restarting from scratch.2. Background GPU abort survival + summary resume/redo correctness
DakePeng/mlx-swift, branchloqi/0.31.4-poison-events): upstream 0.31.4 plus the ml-explore/mlx#3523 backport (tseylerd/mlx#1). Stock mlx throws C++ exceptions from Metal completion handlers when iOS aborts GPU work at background entry — uncatchable, killed the app every time a summary was backgrounded mid-generation (or froze it under the debugger). With the fork, aborts poison the stream and surface as catchable errors. Drop the fork once mlx-swift vendors mlx ≥a025496c.UIApplicationbackground task so jetsam can't lose the last checkpoint, and progress reporting counts cached chunks so the UI never appears to reset after backgrounding.3. Map-phase extraction hardening (Qwen3.5-2B format drift)
Driven by privacy-safe device diagnostics (rejection tallies, never content): per-line adaptive TSV field splitting, a localized one-shot example (kills spec parroting), a forced response prefix (
T\t<chunk>\t) so the first record line is well-formed by construction, tolerant chunk-id matching with content-based echo guards, a strictly-gated salvage pass for compressed/id-led schemas (grounding via source-id validation preserved), and m-id citation scrubbing at parse/reduce/render boundaries.Reviewer notes
project.pbxprojadds a root-level SPM override of the transitiveml-explore/mlx-swift. The fork branch is 0.31.4 + one submodule-pointer commit; the mlx patch is the upstream-merged fix backported to the vendored SHA.🤖 Generated with Claude Code