Skip to content

fix: robustness cleanup — error handling, resource leaks, config hygiene#3

Open
TuurHart wants to merge 1 commit into
mainfrom
robustness-cleanup
Open

fix: robustness cleanup — error handling, resource leaks, config hygiene#3
TuurHart wants to merge 1 commit into
mainfrom
robustness-cleanup

Conversation

@TuurHart

@TuurHart TuurHart commented Apr 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • Backend: Replace bare except: and print() calls with proper logging across 5 files (system.py, files.py, transcription.py, sanitisation.py, enhancement.py). Errors that were silently swallowed now surface in logs.
  • Frontend: Fix EventSource leak on sidebar unmount, add cancellation support to transcription polling, clean up polling intervals on unmount, remove debug console.logs from drag-drop handler, fix non-null assertions.
  • Electron: Add backendProc.on('error') handler so spawn failures surface immediately instead of a silent 60s timeout.
  • Gitignore: Stop tracking user-specific files (user_settings.json, names.json, batch_state.json) — they stay on disk but are no longer in the repo.

Test plan

  • Start backend — no import/startup errors
  • Start frontend (npm run dev:electron) — app loads normally
  • Upload a file, transcribe, enhance — full pipeline works
  • Check Electron dev console for leaked EventSource or interval warnings
  • npx tsc --noEmit passes (verified locally)

🤖 Generated with Claude Code

…king

Backend: replace bare except clauses and print() with proper logger calls,
log ffmpeg pass-1 failures, warn on names.json parse errors.

Frontend: fix EventSource leak on sidebar unmount, add cancellation support
to waitForTranscription polling, clean up transcription polling interval on
unmount, remove debug console.logs from drag-drop, fix non-null assertions.

Electron: surface backend spawn errors immediately instead of silent 60s timeout.

Gitignore: stop tracking user_settings.json, names.json, and batch_state.json.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TuurHart added a commit that referenced this pull request Jun 15, 2026
…ibe (conv bug-hunt #1#3,#5)

Fixes the conversation/diarization → name-linking → Obsidian export defects on the
Mac (CONVERSATION_BUGHUNT_PROMPT.md). User decisions locked; see backlog.md.

- #1 over-canonicalization: inline name mentions now link as Obsidian alias-display
  `[[Canonical|spoken]]` (every mention) — the SPOKEN word ("Tuur") is preserved, not
  replaced by the canonical/short. New `Sanitiser.processConversation`.
- #2 speaker labels: a speaker's FIRST turn header → full `[[Canonical]]`, every later
  header → plain short name `**Tuur:**` (no link). Fixes the "every header full
  canonical" bug (voice-only people with empty aliases were skipped before the demote).
- #3 fragmentation: `SpeakerTranscript.parse`/`mergeAdjacentTurns` ported to the
  desktop; processConversation merges consecutive same-speaker turns.
- isAttributed line-anchored + requires ≥2 DISTINCT speakers — a hand-formatted
  `**Pros:**`/`**Cons:**` body is no longer mis-read as a conversation (was skipping
  copy-edit on plain notes).
- BatchRunner: conversations route to processConversation; the Mac-diarize path now
  emits PLAIN headers so phone-synced and Mac-diarized memos render identically.
- Pipe-aware link identity (the regression trap from introducing `|alias`): linkTarget
  / hasCanonicalLink / linkDisplay; BodyTextView.person(matchingCore:); resolver
  first-mention detection; unlink/relink restore the spoken display word.
- #5: Re-transcribe + Redo-copy-edit hidden for diarized memos (would destroy the
  turns — the phone uploads no segments/word-timings, so the `**Name:**` text is the
  only copy); redo(.copyEdit) keeps a conversation verbatim defensively.

Gate: 255 UnitTests green (+7 new conversation tests); full SkriftDesktop build
(-skipMacroValidation) SUCCEEDED.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TuurHart added a commit that referenced this pull request Jun 15, 2026
… turns (conv bug-hunt #3,#4,#5)

- #5 data-loss: MemoSaver.diarizeIntoTurns marks the diarized transcript
  transcriptUserEdited=true. A phone conversation's `**Name:**` turns are the only
  copy (segments/word-timings aren't uploaded); without this, a noisy multi-speaker
  take with ASR confidence < 0.7 was UNtrusted → silently re-ASR'd on the Mac at
  ingest, destroying the turns. Same rationale as the quote-capture / append paths.
- #3/#4 fusion hardening (SpeakerFusion, byte-identical on both apps):
  - minTurnWords default 2→3: a 1–2 word wrong burst flanked by the same speaker on
    both sides is now smoothed away (less fragmentation / mid-run mis-attribution).
  - gap-word assignment uses the NEAREST SEGMENT BOUNDARY, not the nearest midpoint —
    a word at a long segment's edge was being snapped to a shorter neighbour whose
    midpoint happened to be closer. (#4 boundary-word mis-snap.)
  Mid-sentence mis-splits remain bounded by Sortformer segment quality; the Mac
  same-speaker merge (processConversation) + manual reassign are the backstops.

Gate: mobile 383 unit tests green (+2 fusion cases); desktop 255 UnitTests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TuurHart added a commit that referenced this pull request Jun 15, 2026
… [[img]] marker

Two of the three reported video-from-Photos bugs (the third, playback, landed
separately):

- Source glyph (#3): a video import is neither a share-capture (it HAS audio)
  nor a book-capture, so it carried no source marker and looked like a plain
  voice memo. Added MemoMetadata.sourceType (free-form String, additive/optional,
  value "video" via MemoMetadata.Source.video; set in processVideo incl. the
  no-audio-track failure path) → Memo.isVideoImport → a video.fill leading glyph
  + a "Video" chip in the list row AND the detail header chips. Mobile-only: NOT
  added to UploadMetadata, so the Mac contract is byte-unchanged (the glyph is a
  phone concern; the full cross-app taxonomy stays deferred).

- Row snippet (#2 real wart): the untitled-row snippet showed the raw
  "[[img_001]]" marker — a video transcript always opens with the frame marker,
  which filled the whole snippet line. MemoCard.snippet now strips [[img_NNN]]
  markers (titled rows already used the marker-stripped firstTranscriptLine).

Thumbnail aspect (#2 literal claim): NOT reproducible — verified via -seedVideoMemo
(a landscape 16:9 frame with a centered circle stays a perfect circle, not an
ellipse, in BOTH the row thumb and the detail embed). The square thumb is a
conventional center-crop, not a distortion; every display path already
aspect-fills + clips. Added VideoMemoUITests (seeds the frame, asserts the Video
chip, screenshots row + detail to /tmp/skrift-video-shots) as the gate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TuurHart added a commit that referenced this pull request Jun 22, 2026
…e + doc refresh

Cleanup pass after the audit (adversarial review + hygiene + doc-consistency):

- Reconcile non-blocking (correctness, the audit's one moderate finding): the launch /
  app-active / CloudKit-import triggers now dispatch the sweep as `Task { @mainactor }`
  instead of a synchronous call, so they never block App.init() or the notification handler —
  the window renders first. SwiftData stays on the main actor by design (the codebase funnels
  all SwiftData onto main to avoid cross-context corruption; a background context was the wrong
  fix). The sweep dedups, so only a one-time large first-enable backlog could jank main —
  off-main blob materialization noted as a deferred follow-up.
- .gitignore: add build-release/ (a device/release derived-data output dir, same as the
  already-ignored build/ and build-device/) so it stops showing as untracked.
- Docs refreshed to match reality: MAC_CLOUDKIT_PLAN.md RESUME (all pushed; device-verified —
  73 memos synced, 32-byte APNs token; remaining = one Process→write-back round-trip + prod
  promotion) + hard-steps #1-#3 marked DONE for dev (capability, team signing, push). FEATURES:
  the CloudKit-push row flipped 🟡→✅ for BOTH apps (mobile since 53451a6, desktop since 6642af4,
  device-verified), and the reconcile row notes the async triggers + device-verification.

Audit cleared everything else: container isolation, gating, contexts, write-back idempotency/
no-orphan, Memo.make round-trip, dedup, no force-unwraps/fatalErrors, phone push fully ready.

Gate: desktop full -skipMacroValidation build SUCCEEDED (reconcile change is App-only).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant