First-launch download progress, and an optional AI key in onboarding - #1
Merged
Conversation
Never push directly to main; branch, commit in logical units, merge deliberately. The reason is reverting: a feature on its own branch, built from small self-contained commits, backs out cleanly — a 2,000-line commit already on main cannot be unpicked without collateral damage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A determinate first-launch progress bar needs two numbers nothing stored before this, both added as nullable columns on `accounts`. `backfill_total_estimate` is the denominator — the `resultSizeEstimate` Gmail returns on the FIRST page of a run. Deliberately the WINDOWED estimate, matching the 90-day `after:` filter the run itself lists, never `profile.messagesTotal`: a whole-mailbox total would leave a decade-old account's bar frozen near zero forever. `seedBackfillProgress` guards both writes with COALESCE so stickiness is enforced in SQL — Gmail returns a different estimate on later pages, and letting one move the denominator would make the bar jump backwards mid-download. `backfill_count_baseline` is what keeps the numerator honest across a RESTARTED backfill. The numerator is a live windowed COUNT, and nothing that restarts a backfill deletes mail: the §4.3 expiry branch and v10 reset only the state, and `deleteAccount` deliberately leaves every message on disk. So a re-list starts with the count already at or above a fresh estimate. Recording the count as it stood when the run STARTED lets the UI tell a first download (baseline 0) from a re-list (baseline > 0) and show a bar only for the former. `restartBackfill` resets state and forgets both seeds in ONE statement. Separately it left a window the COALESCE guard made permanent: a crash between the two writes left a run marked pending whose sticky seeds could never be overwritten, so it re-listed forever under stale numbers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`BackfillProgress` carries raw numbers, not a percentage: the ratchet and the ceiling are presentation decisions belonging to the view model, which is also the only layer that knows what it showed last. Store reports what is true right now. `isRunning` follows persisted STATE, never arithmetic. A page whose messages 404 between `list` and `get` is skipped without incrementing the count, so an arithmetic completion test would hang below 100% forever on any mailbox with deleted mail. `isFirstDownload` is strictly `== 0` and `isSeeded` is separate, so an unseeded run (nil, between a restart and the next first page) is never mistaken for a fresh one — during that window `stored` already reflects a full mailbox. `observeBackfillProgress` re-emits on the existing per-page backfill commit, so the bar advances a page at a time with no polling and no new sync path. `removeDuplicates` because hydration and triage also write `messages`; without it every body fetch would churn the UI during the exact phase this exists to smooth. A missing account row reads as complete rather than throwing — a failed stream is of no use to the UI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`MessageListPage.resultSizeEstimate` was decoded and thrown away. It is the only number Gmail gives us scoped to the SAME windowed listing the backfill performs, which is what makes it an honest denominator — so the first page of every run now seeds it, along with the count baseline. `backfillWindowStart` becomes the single source of truth for the window boundary: `backfillQuery` turns it into the Gmail `after:` filter and the progress observation counts stored messages from the same instant. Two independent expressions of "90 days before consent" would drift, and the bar would never reach its denominator. The §4.3 cursor-expiry branch now calls `restartBackfill`, which resets the run and forgets the previous run's seeds atomically. Carrying the old denominator into a re-list would compare a fresh listing against a count that already includes everything it is about to re-list — the bar would open at its ceiling and sit there for the whole run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reported bug. On a freshly connected account the sidebar footer read "All synced" and the inbox read "No messages here" while hundreds of messages were still downloading, then everything landed at once. `isCatchingUp` was declared `false` and first assigned only AFTER the opening `syncOnce()` returned — hundreds of `messages.get` calls later. It is now seeded in both initializers from the persisted `backfill_state` already sitting in the `AccountRecord` we hold: no network, no Keychain, no await. Every async path may only refine that value, so the very first frame is already honest. Three further honesty fixes in the same signal: - A thrown pass no longer reads as "nothing left to do". `try?` yields nil and `Optional(nil) == Optional(false)` is false, so one network blip used to flip the footer to "All synced" AND relax the poll interval. `isSyncStalled` now says "Waiting for network…" instead, and `syncNow` shares the same flag so a successful manual sync clears it. - `refreshHydrationBacklog` and the progress subscription are hoisted ABOVE the `guard let stack`, and the nil-stack path clears the flag and banners. Everything that can clear `isCatchingUp` used to live below that guard, so a credential-less launch latched it on forever. - A `Task.isCancelled` check before the loop's state writes, so a pass in flight during a disconnect cannot repopulate what the disconnect just cleared. The fraction ratchets (never rewinds when a deleted message shrinks the live count), caps below 1.0 while running (the denominator is Gmail's own approximation), and fills only a bar that was actually drawn — completion is signalled by state, never by arithmetic. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`InboxListView` branched on `inbox.rows.isEmpty` alone, so a mailbox still downloading and a mailbox with nothing in it rendered identically as "No messages here" — the other half of the reported bug. `rows` starts `[]`, which is indistinguishable from an empty mailbox, so `InboxModel` gains `hasLoaded` — false until the row observation has actually answered. The empty state is now three-way: blank before the store has answered (a label that appears for two frames and vanishes is noise), "Getting your mail…" while catching up, "No messages here" otherwise. `hasLoaded` is deliberately never reset, not even when the folder or split changes: `rows` retains the previous emission across a re-subscribe, so there is no empty frame to protect there and resetting would flash a loading state on every tab click. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ProgressTrack` rather than `SwiftUI.ProgressView`: the macOS style brings its own accent color, radius, height and — for the indeterminate case — a perpetual animation, none of which are expressible in Hudson's tokens. A nil fraction renders an EMPTY TRACK, never a fake. The total is genuinely unknown before Gmail's first page returns, and a re-list of mail already on disk is not progress the user can see; both get a quiet empty slot beside the status line. That choice is also why this needs no `.repeatForever` and no new Motion token — an indeterminate sweep would have required both, plus a carve-out from the 280ms budget every other animation here respects. The footer slot is reserved permanently rather than inserted when a backfill starts. The status line above already pins its own height so the divider never moves; animating the footer's height would shift that divider at the one moment the app is trying to look composed. Wording never states the estimate as an exact denominator — "of about N", dropping the total entirely once the real count passes it. Gmail documents `resultSizeEstimate` as approximate, which is the same reason the bar is capped below 100%; claiming "340 of 1,240" would assert a precision the number does not have. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pasting an API key and agreeing that your email may be sent to that provider are two different decisions. `save(optIn:)` lets onboarding take the first without the second; the Settings sheet keeps the default `true`, where its button genuinely does say "Enable AI". Opt-in is refused when the provider is not actually configured, and the banner now reports what was WRITTEN rather than what was requested — saying "AI enabled." while persisting `opt_in = 0` would be a worse lie than the one this change set exists to remove. That refusal is where the real bug was. Waiving validation for `.openAICompat` because it may legitimately be keyless also waived the ENDPOINT: an empty Base URL encodes as bare "openai-compat", which `AIBootstrap` decodes to `baseURL: nil`, and `OpenAICompatProvider` then falls back to `https://api.openai.com/v1`. The one provider choice whose entire premise is "nothing leaves this machine" was the one that could opt in with no key, no endpoint and no model — silently pointed at a cloud host. It now requires a parseable host and a model instead of a key. `revokeAIOptIn` flips `opt_in` only, leaving `model` and `base_url` untouched. `disable()` used to re-write every row with `model: ""`, `baseURL: nil`, erasing the provider encoding — so a user running a LOCAL model, whose whole point is that nothing leaves the machine, silently came back as cloud Anthropic when they re-enabled. Neither call is `try?`'d: consent lives in the database, which `AIBootstrap` reads and `isEnabled` does not, so a swallowed failure would leave every feature genuinely opted in while the UI reported the opposite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ai_config` has no foreign key to `accounts`, so its rows outlive the delete. Reconnecting the same address later silently restored an `opt_in = 1` granted in a previous session, and the first Summarize tap would egress against consent the user has every reason to believe they revoked. That path stops being theoretical now that the onboarding AI step makes reconnect routine. Revoked BEFORE deleting the account row, mirroring the existing Keychain-then-Store reasoning: a failure here leaves everything intact and retryable, whereas revoking last would let a crash between the two writes orphan `opt_in = 1` on an address the app has already forgotten — precisely the resurrection this closes. Not `try?`, for the same reason the other two purge steps aren't: a partial purge must never be reported as a completed disconnect. Also tears down the progress subscription and clears the sync flags, so a disconnected mailbox stops claiming to be downloading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`onConnected` is what mounts the mailbox and starts the sync loop. If the key step sat in front of it, backfill would not begin until the user finished — and for the Anthropic path "finishing" can mean leaving the app entirely to go create a key, with nothing downloading the whole time. So the flow splits in two. `onAccountPersisted` fires the moment the account row exists, letting the host start syncing behind the still-showing step; `onConnected` fires when the step resolves and only dismisses onboarding. By the time the user skips or saves, mail is already arriving. `finishAIKeyStep` is the single exit — Save and Skip both land there — and a host that never wires the persisted hook falls straight through to `.done`, so existing callers and previews are unaffected. The screen is optional and says so; Skip is a peer of Save, not a footnote. Three consent decisions it has to get right: - The toggle is disabled until a key is typed. `LLMKeyStore` is scoped per PROVIDER, not per account, and disconnecting never removes the key — so without this the step could grant `opt_in` for a brand-new account against a credential left by a previous one, which its own empty field never displays. - Nothing is written when the user typed nothing and left the toggle off. `save` unconditionally rewrites all four `ai_config` rows as Anthropic, so calling it for an empty form would overwrite a returning address's provider — the same local-to-cloud downgrade `revokeAIOptIn` exists to prevent, arriving from the other side. It `load()`s first for the same reason. - The explainer names BOTH things the single toggle grants. An earlier draft said "only the thread or question you explicitly act on leaves your Mac", which was untrue: this consent also covers `.voiceProfile`, and `VoiceProfile.generate` reads a sample of SENT mail. Consent copy that understates the grant is worse than no copy. The phase and its screen land together because Swift requires the switch over `Phase` to stay exhaustive — they are one unit, not two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An adversarial review caught this before it shipped. `bootPhase` had been reordered to give onboarding precedence over a live model, but `body` re-tested `model`/`onboarding` inline in the OLD order, and `bootPhase` was consumed only as the `.animation(value:)` key. Nothing failed loudly — the AI-key step was simply never mounted, so the feature was dead on the real boot path. Worse, `onboarding` then stayed non-nil forever (its only exit is a button on that unmounted screen), permanently no-oping the reverse gate, and after a disconnect the stale screen resurfaced and could rebuild an `AppModel` for the deleted account. `body` now switches on `bootPhase`, and the precedence rule is extracted to a pure `resolveBootPhase(hasOnboarding:hasReadyMailbox:)` so it can be asserted on: `@State` cannot be read back off a View value a test constructs, so a rule left inline is a rule nothing can cover — which is exactly how this got through. Onboarding outranks a ready mailbox because between `onAccountPersisted` and `onConnected` both are live at once: the account exists and its mail is already downloading, while the optional key step is still on screen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Regression coverage for the two defects a user report and an adversarial review found, plus the arithmetic that has to hold around them. - `aFreshlyConnectedAccountIsCatchingUpOnTheVeryFirstFrame` — constructs an `AppModel` with no await between construction and the assertion, which is exactly what the original bug failed. - `onboardingOutranksAReadyMailboxWhileBothAreLive` — the boot precedence the review found inverted, covering the both-live state that made the AI-key step dead code. - The bar cannot rewind, exceed its ceiling, divide by zero, or acquire a fill on a run that deliberately has none; a re-list over mail already on disk shows no bar rather than one pinned at 95%. - Footer wording hedges the estimate, drops it once exceeded, and yields to "Waiting for network…" when a pass throws. - Storing a key does not opt in; opting in without a usable provider is refused; disabling preserves a local model's base URL; disconnecting revokes consent so a reconnect cannot resurrect it. - Sync starts at `onAccountPersisted`, before the key step resolves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mannasdev
added a commit
that referenced
this pull request
Aug 17, 2026
… not Hudson 0.1.0 and 0.1.1 crashed on launch on every Mac except the one that built them, before drawing a single frame: 0 libswiftCore _assertionFailure(_:_:file:line:flags:) 1 HudsonApp closure #1 in variable initialization expression of static NSBundle.module 5 HudsonApp specialized static Typography.register() 6 HudsonApp specialized static Typography.serif(_:_:) 7 HudsonApp closure #1 in RootView.loadingPlaceholder.getter SwiftPM's generated `Bundle.module` probes exactly two paths and calls `fatalError` when both miss: the app bundle ROOT — `Bundle.main.bundleURL`, not its `Resources` directory — and an absolute path into the `.build` directory of the compiling machine. A packaged .app matches neither. codesign requires the nested resource bundle to live in Contents/Resources, which is not the root, and a user's Mac has no .build directory. So the lookup succeeded only on the maintainer's machine, via a path that shipped inside the binary as a string. Locate the bundle by hand across the three layouts that actually occur — a packaged .app, `swift run`, and the test runner — and never trap. Typography was already built to degrade: `resolved(_:size:weight:fallback:)` falls back to system faces when a family will not resolve. `Bundle.module` was the one call that turned a missing font into a dead app. This also fixes a second bug the crash was hiding. SwiftPM FLATTENS `Resources/Fonts/*.ttf` into the bundle root, so the old `subdirectory: "Fonts"` argument matched nothing and no bundled face has ever registered — the app has been rendering in system fonts on every machine, including this one. Both layouts are probed now. The tests drive the resolver against real on-disk layouts rather than the ambient one, because the ambient layout is what hid this: the dev loop resolved, so nothing exercised the shape a user receives. Co-Authored-By: Claude Opus 5 (1M context) <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.
Two user-reported gaps, plus the defects an adversarial review found in the first cut of both.
1. The app lied on first launch
On a freshly connected account the sidebar footer said "All synced" and the inbox said "No messages here" while hundreds of messages were still downloading — then everything landed at once.
Root cause:
AppModel.isCatchingUpwas declaredfalseand first assigned only after the openingsyncOnce()returned. It is now seeded synchronously in both initializers from the persistedbackfill_statealready in hand — no network, no await — so the very first frame is honest. Async paths may only refine it.The inbox list gained the matching distinction:
InboxModel.hasLoadedseparates "the store hasn't answered yet" from "genuinely empty".The progress bar
Migration v11 adds two nullable columns on
accounts:backfill_total_estimate— the denominator, from the first page'sresultSizeEstimate. Windowed, matching the 90-dayafter:filter the run actually lists. Neverprofile.messagesTotal, which would leave a decade-old account frozen near zero. Sticky viaCOALESCE, because Gmail returns a different estimate on later pages.backfill_count_baseline— the windowed count when the run started. Nothing that restarts a backfill deletes mail (deleteAccountdeliberately keeps it; §4.3 expiry and v10 reset only state), so a re-list begins with the count already at or above a fresh estimate. Without this the bar renders pinned at 95% for the whole re-list.A bar shows only for a genuine first download. Re-listing mail already on disk isn't visible progress, so it gets the quiet status line. The fraction ratchets, caps below 1.0 while running (the denominator is Gmail's own approximation), and completion is signalled by state — never arithmetic, since a page whose messages 404 between
listandgetnever increments the count.Wording never claims false precision: "340 of about 1,240", dropping the total once the real count passes it.
isSyncStalledsurfaces "Waiting for network…" so an offline launch explains itself instead of freezing.2. AI key in onboarding
A new skippable step. Sync starts at
onAccountPersisted— the moment the account row lands — so the step costs zero mail-download time; by the time you skip or save, mail is already arriving.Storing a key and consenting to send mail to a provider are separated (
save(optIn:)). Three consent bugs fixed along the way:openai-compatcould opt in with no key, no endpoint and no model — an empty base URL decoded tonilandOpenAICompatProviderfell back tohttps://api.openai.com/v1. The one provider whose premise is "nothing leaves this machine" was silently pointed at a cloud host.opt_in = 1behind (ai_confighas no FK toaccounts), so reconnecting resurrected consent. Now revoked before the account delete, and not swallowed.disable()wipedbase_url, silently downgrading a local model to cloud Anthropic on re-enable.The consent copy also now names both things the toggle grants — an earlier draft claimed only the acted-on thread leaves your Mac, but the same consent covers
.voiceProfile, which reads a sample of sent mail.Review
An 8-agent workflow designed it (4 blockers found pre-implementation), then a 32-agent workflow reviewed the diff: 28 candidate findings, 20 survived adversarial verification. All fixed, including a blocker that made feature 2 dead code —
bodyre-tested the boot conditions inline in the old order whilebootPhasewas only the animation key, so the AI-key screen was never mounted.bodynow switches onbootPhase, and the precedence rule is a pure function so it can actually be tested.Verification
swift buildclean; every one of the 12 commits builds independently (checked out and built each)swift test— 743 passing, up from 697swift run HudsonApp --demolaunches clean🤖 Generated with Claude Code