Skip to content

Slice/m3.1b embedder lifecycle - #401

Merged
failerko merged 83 commits into
mainfrom
slice/m3.1b-embedder-lifecycle
Jul 29, 2026
Merged

Slice/m3.1b embedder lifecycle#401
failerko merged 83 commits into
mainfrom
slice/m3.1b-embedder-lifecycle

Conversation

@failerko

@failerko failerko commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features
    • Added a Memory panel in Story Settings with stale-status visibility, reindex/model swap/relabel actions, and inline resume for pending swaps.
    • Introduced embedder swap and swap-resume dialogs, plus provider-based embedding-dimension detection to support effective-dimension memory-cost configuration.
    • Enabled automatic background processing to drain incomplete embeddings.
  • Bug Fixes
    • Improved generation status pill errors (including memory-incomplete and swap-paused) with updated actions/tap behavior.
    • Added stricter validation and clearer messaging for invalid custom effective dimensions.
  • Tests
    • Expanded UI, unit, and end-to-end coverage across memory, embedding drain, swap/resume, and wizard flows.

failerko and others added 30 commits July 24, 2026 17:13
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tables

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ging

pk carries branchId:id:modelId so phase-1 staging coexists with old-model rows; all deletes go by real columns so pre-widen rows stay forward-compatible with no destructive migration.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds three real-DB tests in ops.test.ts: a legacy 2-part-pk row is matched by columns and replaced by upsert / removed by deleteVecOps, and two models coexisting on one (branch, id) leave both rows with the first model's source_hash untouched. Also drops phase-numbering wording from the vecRowPk comment and notes why deleteBranchModelVecOps is model-scoped vs deleteBranchVecOps's all-model sweep.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Truncation lives inside embedTexts (post-normalize-check, pre-normalize) so every consumer inherits it transparently; resolveEmbedderConfig threads story.effectiveDim into EmbedderConfig and gates requestDimensions on opts.matryoshkaSupported. Truncation uses .slice (independent copy) before re-norm, never .subarray, since l2Normalize can return its input untouched and packFloat32 packs the backing view. gate.test.ts collaterally updated for the widened provider config shape.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The native-dim mismatch guard rejected the happy path where a compliant provider (e.g. OpenAI text-embedding-3-*) honors the `dimensions` param and returns effectiveDim directly instead of native: raw.dim !== config.dim threw even though the vector was correctly truncated. Now accepts raw.dim === effectiveDim as well, but only when requestDimensions was actually sent, so a provider returning some other dim still fails loudly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
marker transitions per retrieval.md Model swap UX; cancel re-flags embedding_stale so rows whose flag cleared during staging revalidate by source_hash; relabel rewrites vec identity in SQL because model_id is now in the pk (no recompute); phase-2 delete skipped for same-model re-index.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
single-flight is a caller precondition (advisory JS guard only); relabel deletes leftover target rows before INSERT to avoid a vec0 pk-constraint rollback; SOURCE_TABLES now shared from the db barrel; db/exec dropped from SwapDeps.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dep-injected controller drains embedding_stale rows between turns when no
pipeline run is active: 5s/30s/120s backoff on embed failure, per-batch yield
so it never begins while a run holds the pipeline. Never surfaces errors, the
blocking pre-retrieval sync stage owns correctness.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Composes SwapDeps and exposes the swap/reindex/relabel/cancel actions under a per-story single-flight lock (SwapBusyError) with guarded store callbacks; relabel refuses while a swap marker is set, and currentModelId always reads the story's current recorded id. Adds a positional-row queryRows seam and wires the drain controller to run_complete idle events plus a story-open kick.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cancelRequested is global; a cancel that ended with no running loop left it set, so the next start/resume/reindex saw it on its first batch poll and silently cancelled an empty swap. Clear it at runStagingSwap entry and in cancelStorySwap's finally. Also documents the native queryRows column-order contract.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A stop() while a drain awaited embedRows still let the catch re-arm a backoff timer, leaking a zombie retry loop next to a freshly-wired controller on HMR / re-boot. Gate the catch reschedule and the batch loop on a stopped flag. Also logs the swallowed cancel-flag read, single-sources the embed composition, and documents the open-branch drain scope.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two-pane SwapDialog (pick candidate → re-index/keep/relabel) and single-pane SwapResumeDialog for the crash-safe model-swap flow in retrieval.md; both are dumb prop-driven compounds with no store reads, ready for Task 10 to wire into Memory panel and reader route. testIDs (swap-reindex/keep/relabel, swap-candidate-<id>, swap-resume/cancel-swap) are the tier-3 E2E contract Task 12 depends on. Pick-pane selection resets via a useEffect keyed on `open` so a reopen never resurrects a stale target.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
keepHint now conveys Keep's persistent effect (stops the upgrade nag until the next manual swap) instead of describing it as a Cancel; relabelDisclaimer restores second-person framing per canon. OptionsPane gets an AlertDialogDescription so screen readers get more than the title. CandidateRow drops the dead cursor-not-allowed class (unreachable behind pointer-events:none). New ReopenResetsPickPane story exercises the load-bearing useEffect(open) reset via a stateful harness that actually closes and reopens the dialog.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the Memory tab (model-management.md → Staleness UI): stale-count summary, a resolvable-reason line derived from resolveEmbedderConfig against the panel's own settings prop (not storiesStore, so it stays testable and can't drift from what's rendered), switch/re-index/resume/cancel actions wired to the embedder engine, and the SwapDialog/SwapResumeDialog surfaces. buildDrainController now self-attaches the Task 8 drain-status sink to a new tiny embeddingStatusStore, and runStagingSwap/cancelStorySwap refresh it after every swap completion or cancel; refreshEmbeddingStatus never throws so it's safe to await inside the exclusive-lock path. The reader pill and a resume-prompt dialog read the same store for the open story.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lifts GenerationStatusPill's hardcoded phase/error/cancel copy into locales/en/common.json (chrome.generationStatusPill), now in scope since this slice activates the pill's error surface. refreshEmbeddingStatus gets a global request-generation guard so a slower, earlier-issued call can't clobber embeddingStatusStore's single slot after a newer call already resolved (per-story counters wouldn't catch cross-story clobbers, which is the actual failure mode). Reader's swap-progress selector narrows to progress?.storyId instead of the whole object, so an embed batch tick on any story no longer re-renders the whole reader. Adds Memory panel stories for the modelMissing and retrying reason branches (previously only providerUnconfigured was covered).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eDim

Effective dim flows through buildStorySettings into the Finish transaction; a Finish gate rejects a non-positive stored dim, and story-config-schema tightens effectiveDim to a positive int so a bad dim can't truncate vectors to garbage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The pre-selection now gates on a persisted effectiveDimTouched flag instead of a mount-scoped ref, so the platform suggestion fires once per wizard session; a deliberate Native (null) choice on a mobile-suggested ladder stays chosen across step-nav remounts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the wizard.md-mandated Custom-dim quality-cliff caveat next to the custom input, and routes per-row previews through the storagePreview key so they render the canon ~{size} / 30 ch format instead of a bare size.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A mid-session app-default swap to a non-Matryoshka model/backend left the hidden disclosure's stale dim in working state, and the embed service truncates on effectiveDim regardless of the flag; Finish now re-resolves capabilities and drops the dim to native when not applicable. Also adds a radiogroup role to the ladder and fixes two comments.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A synthetic idOverride model id resolves no catalog entry, so resolveEmbedderConfig rejects it for a local-backend story — the swap dialog's re-index path can't target it and relabel is the swap-engine path the copy drives; re-index instead runs standalone while the story is still on the catalog model, where it exercises the full staging engine.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Split re-index and relabel assumed the first test's navigation and DB state, which a retry — a fresh app at Home replaying only the failed test — would not reproduce; one body keeps the ordering they genuinely need.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Boolean-collapsed polls reported only 'false' after 90s; they now poll a shape with a message so a timeout names the condition and prints its actual.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A local packaged run against a stale release/ artifact fails only the specs newer than the bundle, which reads as a packaged-runtime bug rather than a missing build step.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Switch-embedder and reindex-now now also disable on live swap progress, not just the settings marker, which only becomes store-visible at swap end. The drain status sink now drops writes for a story the user has navigated away from, matching the guard refreshEmbeddingStatus already applies via its own generation counter. Removed the unused embeddingStatusStore.clear and the SwapProgress barrel re-export.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
failerko and others added 17 commits July 28, 2026 12:18
sameModel was model-id-only, but a vector space is keyed on (model_id, dim) — swapping a story between a provider copy and a local copy of one model changes the family while the id stays put, so the old family survived the flip as orphans retrieval can never reach. Phase 2 now sweeps every family except the one staging actually landed on, which the service reports since it clamps the declared dim to native.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The swap lock is per-story, so two stories can legitimately stage at once, but progress was one global slot: a second story's beginProgress cleared the first's pending cancel, isCancelRequested ignored the storyId makeCallbackGuards already threads, and a run on any story suppressed every other story's resume prompt. The slot is now keyed by story and every mutator scopes to its own run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Emptying the custom field leaves an unparseable draft while the store keeps the last valid dim. With that dim on the ladder, a step-nav remount renders the ladder pick and no custom input, so the flag survived to block Finish over an error message nowhere on screen. It now clears whenever the custom row is not the active choice, and stays set while the bad field is visible.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
markerTarget is the only code turning a persisted marker back into a target, and neither resumeStorySwap nor cancelStorySwap's recovery branch was reachable from any test — reverting it to the pre-fix shape wedged a story with no resume, no cancel and no drain while the whole suite stayed green. Covers cross-backend resolution both ways, the absent-backend legacy marker, the no-marker refusal, and reindex keeping the story's own backend.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…gging

Cross-model staging writes under the target's own model id and overwrites nothing, so the old vectors a cancel reverts to are still there — but staging had cleared their flags, and restoring all of them queued a full re-embed, on a paid provider, of exactly the work the user cancelled. The flag is now re-derived per row from the stored source_hash, so a row edited mid-swap still comes back stale. recomputeStaleOp becomes set-based and gains its first caller, retiring its dead-code triage entry; it discovers the dim family rather than taking one, since a provider's native dim is not knowable here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
swap-paused existed to carry the one state the stale count cannot raise, and nothing rendered or exercised it: no story, no E2E, and the pill's stories had no play at all, so onErrorTap was never asserted for any code. The E2E takes the real discovery path — story open prompts, Later defers, and the pill is then the only signal left — and asserts it outranks the seeded stale rows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The commenting rule bans prior-approach narration, task references and repeated prose; a slice this long accumulated all three, and a mid-branch hygiene pass had itself added more. Also corrects two comments the code had outgrown: the pill no longer describes every cancel as re-flagging, and SwapCancelOutcome documents its three members rather than three situations that do not map onto them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A settings save merges the whole blob in its own transaction, so one landing during phase 1 can carry a pre-swap snapshot the flip then overwrites — vectors re-embedded under the new model, flags clean, settings still naming the old one, and nothing that re-derives staleness. The preflight now re-reads the marker, turning that silent total loss into a resumable failure. Does not close the race; the triage entry keeps it and now records the grown blast radius.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Batch-level failure told us a batch was bad, never which row, so one un-embeddable row kept its ~15 batch-mates undrained forever at the 120s cap. A second consecutive failing pass now re-runs the batch row by row: the innocent rows land and the offender is quarantined. Isolation waits for that second pass because a provider outage fails every batch and would otherwise cost BATCH_SIZE single calls a cycle, and a batch where every row fails alone quarantines nothing — that is the embedder, not the content. The set is runtime-only, dropped on story switch and on any explicit kick.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same-model staging skips no rows, so the null case is not a resume that found everything staged — it is a story with nothing embeddable, where there is equally nothing to sweep.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Relabel rewrites model_id in place and never moves a row between dim families, so it only holds while the target is read at the dim the vectors already live at. A story truncating to effectiveDim on a provider, relabelled onto the local copy of that model, reads at the full catalog dim and finds nothing — every vector orphaned, silently, on the one path that exists to avoid re-embedding. Re-index is the honest operation there and the toast says so. An unresolvable target still relabels: naming a model the catalog does not know is relabel's job, and an unknown dim simply means no guard.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Truncation is provider-only, so on a local story the dim describes nothing while still reading as a live setting to anything that forgets to check the backend — the shape of bug the relabel dim guard exists to catch. Both the phase-2 flip and relabel write through setEmbeddingTargetOp, so one merge-patch null covers both. One-way for now: dim selection is wizard-only until M7 owns it, so a story that moves to local cannot get its truncation preference back.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same model IDs can span different vector families, so crash recovery records both dimensions and provider-native dimensions are probed before use.
Records the durable provider-dimension and swap-marker contracts. Keeps native-bound validation and custom-import dimensions routed to M7.
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 59934011-32f3-4b84-8c9a-2f9475ca9609

📥 Commits

Reviewing files that changed from the base of the PR and between 7cf2142 and b0066a3.

⛔ Files ignored due to path filters (1)
  • docs/implementation/milestones/03-memory-floor/slices/01b-embedder-lifecycle.md is excluded by !docs/**
📒 Files selected for processing (19)
  • components/embedder/swap-resume-host.tsx
  • components/story-settings/memory-panel.tsx
  • components/wizard/finish.test.ts
  • components/wizard/finish.ts
  • components/wizard/memory-cost-disclosure.stories.tsx
  • components/wizard/memory-cost-disclosure.tsx
  • components/wizard/memory-cost-logic.test.ts
  • components/wizard/memory-cost-logic.ts
  • hooks/use-swap-resume-actions.ts
  • lib/actions/embedder-swap/app-deps.test.ts
  • lib/actions/embedder-swap/index.ts
  • lib/actions/index.ts
  • lib/actions/settings/providers.test.ts
  • lib/actions/settings/providers.ts
  • lib/db/stories/settings-ops.test.ts
  • lib/db/stories/settings-ops.ts
  • lib/embedder/drain.test.ts
  • lib/embedder/drain.ts
  • locales/en/wizard.json
🚧 Files skipped from review as they are similar to previous changes (10)
  • lib/actions/embedder-swap/index.ts
  • lib/db/stories/settings-ops.ts
  • components/wizard/memory-cost-disclosure.tsx
  • lib/embedder/drain.ts
  • locales/en/wizard.json
  • lib/db/stories/settings-ops.test.ts
  • components/wizard/finish.test.ts
  • lib/embedder/drain.test.ts
  • lib/actions/embedder-swap/app-deps.test.ts
  • components/wizard/finish.ts

📝 Walkthrough

Walkthrough

This PR adds provider embedding-dimension discovery, Matryoshka configuration, model swap and relabel workflows, stale-row draining, memory settings UI, resume prompts, status reporting, database indexes, and extensive unit, Storybook, and end-to-end coverage.

Changes

Embedding configuration and wizard

Layer / File(s) Summary
Effective-dimension selection and commit flow
components/wizard/*, app/wizard.tsx, lib/stores/wizard/*, lib/db/stories/*
The wizard exposes validated effective-dimension choices, persists touched selections, validates invalid drafts, and commits effective dimensions for applicable provider models.
Provider probing and truncation
lib/actions/settings/*, lib/ai/*, lib/embedder/*
Provider dimensions are probed and cached, provider configs carry truncation metadata, and embedding requests and vectors honor server- or client-side effective dimensions.

Embedding storage and swap workflows

Layer / File(s) Summary
Vector and stale-row storage
lib/db/embeddings/*, lib/db/runtime/*, lib/db/*table.ts, lib/db/migrations/*
Vector identities include model IDs, stale-row queries and recomputation become set-based, and partial stale indexes plus migration metadata are added.
Swap engine and drain orchestration
lib/actions/embedder-swap/*, lib/embedder/drain.ts, lib/boot/bootstrap.ts
Swap staging, resume, cancellation, relabeling, crash recovery, per-story locking, and idle stale-row draining are implemented with progress and retry handling.

Application UI and validation

Layer / File(s) Summary
Memory and swap UI
components/story-settings/*, components/embedder/*, app/story-settings/*, app/reader-composer/*, lib/stores/ui/*, locales/en/*
Story settings expose memory status, reindex, swap, relabel, cancel, and resume controls; the generation pill reports memory-incomplete and swap-paused states.
Integration coverage and runtime wiring
app/_layout.tsx, e2e/*, components/**/*.stories.tsx, lib/**/*.test.ts
Root hosts, Storybook interactions, unit tests, and Playwright scenarios validate provider probing, memory states, drain behavior, swaps, relabeling, and resume flows.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Poem

I’m a bunny with vectors, hopping in line,
Swapping old memories for models divine.
Stale rows get drained, dimensions grow,
Resume or cancel—now buttons say so.
Tests nibble carrots and status lights glow!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.37% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the PR’s main theme: embedder lifecycle work, and is concise enough for history scanning.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (6)
lib/db/runtime/exec.native.ts (1)

20-31: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift

Avoid synchronous, unbounded native row loading.

queryRows is declared async but calls getAllSync and materializes every result row. The swap workflow uses it to load stale rows for each vector kind, so large stories can block the JS thread and allocate the full result before batching begins. Prefer an async or chunked query path while preserving the positional-row contract.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/db/runtime/exec.native.ts` around lines 20 - 31, Update queryRows to
avoid getAllSync and unbounded full-result materialization by using the
available asynchronous or chunked expo-sqlite query mechanism. Preserve its
Promise-based API and positional-row contract by converting each fetched row
with Object.values() in SELECT column order, while allowing callers to process
stale rows incrementally.
lib/db/stories/settings-ops.ts (1)

73-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing test coverage for setSwapTargetDimOp.

Every other exported op in this file has a dedicated test; this one is exercised nowhere in settings-ops.test.ts.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/db/stories/settings-ops.ts` around lines 73 - 75, Add a dedicated test
for the exported setSwapTargetDimOp function in settings-ops.test.ts, verifying
it produces the expected settings patch for embedding_swap_target_dim and
preserves the provided storyId and nowMs values, consistent with the other
settings operation tests.
locales/en/wizard.json (1)

53-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider parameterizing the chapter-count assumption.

30 is baked into both storagePreview and footer. Passing it as {{chapters}} keeps copy in sync with the estimation constant and avoids the untranslatable ch abbreviation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@locales/en/wizard.json` around lines 53 - 57, Update the storagePreview and
footer locale strings to replace the hardcoded 30 chapter count with the
{{chapters}} interpolation, and replace the abbreviated “ch” with translatable
wording. Ensure the corresponding storage-estimate rendering passes the
estimation constant as chapters so both messages remain synchronized.
lib/actions/embedder-swap/index.ts (1)

1-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Barrel omits SwapMarkerChangedError and the SwapCancelOutcome type.

cancelStorySwap returns SwapCancelOutcome, so consumers outside this folder can't name its result type, and SwapMarkerChangedError (thrown by the phase-2 preflight) can't be caught by type through the public surface while every sibling swap error can.

♻️ Suggested export additions
   SwapNotInProgressError,
+  SwapMarkerChangedError,
   SwapStoryMissingError,
   startStorySwap,
   SwapBusyError,
   SwapConfigError,
+  type SwapCancelOutcome,
 } from './app-deps'
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/actions/embedder-swap/index.ts` around lines 1 - 31, Add
SwapMarkerChangedError and the SwapCancelOutcome type to the public exports in
lib/actions/embedder-swap/index.ts, sourcing each from the module that defines
it alongside the existing swap errors and related types. Preserve all current
exports so consumers can name cancelStorySwap’s return type and catch the
phase-2 preflight error through the barrel.
lib/actions/index.ts (1)

2-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider not re-exporting raw engine primitives from the app-wide actions barrel.

startSwap, resumeSwap, reindexStory, cancelSwap, relabelModel and SwapDeps/SwapParams require the caller to hold the per-story single-flight lock (engine.ts header contract); the safe entry points are the *Story* wrappers that go through runExclusive. Exposing both side by side at lib/actions makes the unsafe pair the more obvious import and orphaned staged vectors the failure mode. Keeping the primitives reachable only via @/lib/actions/embedder-swap (tests) would narrow that surface.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/actions/index.ts` around lines 2 - 29, Remove the raw engine primitives
startSwap, resumeSwap, reindexStory, cancelSwap, relabelModel, and the
SwapDeps/SwapParams types from the lib/actions barrel export. Keep the *Story*
wrapper entry points and error/configuration exports available, leaving
primitives reachable only through the embedder-swap module.
components/story-settings/memory-panel.tsx (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared resume/cancel-swap error-handling into one hook.

MemoryPanel and SwapResumeHost each reimplement the same resume/cancel-swap orchestration (SwapBusyError→toast.info branch, logger.error with the same key shape, generic toast.error, and finally(() => void refreshEmbeddingStatus(storyId))), differing only in whether a dialog is closed/navigated first. A shared useSwapResumeActions(storyId) (or similar) hook would remove the duplication and keep the error-reporting contract in one place as it evolves.

  • components/story-settings/memory-panel.tsx#L150-277: extract reportEngineFailure, handleResume, and handleCancelSwap into the shared hook, keeping only the swap-dialog-close call local to this component.
  • components/embedder/swap-resume-host.tsx#L46-83: extract report, onResume, and onCancelSwap into the same shared hook, keeping only the router.navigate call local to this component.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/story-settings/memory-panel.tsx` at line 1, Extract the duplicated
resume/cancel-swap orchestration from MemoryPanel’s reportEngineFailure,
handleResume, and handleCancelSwap and SwapResumeHost’s report, onResume, and
onCancelSwap into a shared useSwapResumeActions(storyId) hook. Preserve the
SwapBusyError toast.info path, logger.error key shape, generic toast.error
handling, and finally refreshEmbeddingStatus(storyId); keep only MemoryPanel’s
dialog-close call and SwapResumeHost’s router.navigate call local.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@components/compounds/embedder-default-card.tsx`:
- Around line 140-174: The persistProvider flow must prevent stale asynchronous
probe results from writing dimensions after a newer selection is made. Add a
selection-generation or equivalent current-selection check around
ensureProviderEmbeddingDim and its recordProviderEmbeddingDim persistence, and
only allow the probe result to write when it still matches the latest
provider/model selection and pending defaults; preserve the existing save and
warning behavior for current selections.

In `@components/wizard/finish.ts`:
- Around line 64-84: Validate the selected dimension against the current model’s
supported ceiling before committing: update the finish flow around effectiveDim
and the dimension validation in components/wizard/memory-cost-logic.ts (lines
42-51) to reject values above embeddingCapabilities.embeddingDim or the intended
valid ladder value, while preserving native/null behavior for non-applicable
models. Thread the dimension bounds through validateCustomDim or its caller so
stale working-state selections cannot commit oversized dimensions.

In `@lib/actions/embedder-swap/app-deps.test.ts`:
- Around line 662-675: Update the assertion following cancelStorySwap in the
cancellation test to inspect the current cancelSwap invocation via
mock.lastCall?.[1], or reset the mock call history before the test. Preserve the
existing expected swap fields while removing reliance on calls[0] and prior test
execution order.

In `@lib/actions/settings/providers.ts`:
- Around line 57-85: Serialize the read-modify-persist operation in
recordProviderEmbeddingDim with a promise-chain lock keyed by providerId,
ensuring concurrent updates for different models on the same provider execute
sequentially and each reads the latest cachedModels state before updateProvider
persists it. Preserve the existing validation and provider-not-found behavior.

In `@lib/db/stories/settings-ops.test.ts`:
- Around line 194-198: Update the titles of both tests around
setEmbeddingTargetOp to reference setEmbeddingTargetOp instead of the removed
setEmbeddingModelIdOp, while leaving their test logic unchanged.

In `@lib/db/stories/settings-ops.ts`:
- Around line 26-30: Update embeddingTargetKey to encode each providerId and
modelId component with an unambiguous delimiter-safe representation before
composing the key, so values containing colons cannot collide. Preserve the
provider/local prefixes and ensure distinct target components always produce
distinct stable keys.

In `@lib/embedder/drain.ts`:
- Around line 41-44: Update the scheduling logic in schedule and noteIdle so
idle notifications do not clear or replace an already-armed retry timer from the
backoff ladder; preserve the pending retry until it fires. Keep kick’s explicit
recovery behavior unchanged so it can still override the existing timer and
trigger an immediate drain.

---

Nitpick comments:
In `@components/story-settings/memory-panel.tsx`:
- Line 1: Extract the duplicated resume/cancel-swap orchestration from
MemoryPanel’s reportEngineFailure, handleResume, and handleCancelSwap and
SwapResumeHost’s report, onResume, and onCancelSwap into a shared
useSwapResumeActions(storyId) hook. Preserve the SwapBusyError toast.info path,
logger.error key shape, generic toast.error handling, and finally
refreshEmbeddingStatus(storyId); keep only MemoryPanel’s dialog-close call and
SwapResumeHost’s router.navigate call local.

In `@lib/actions/embedder-swap/index.ts`:
- Around line 1-31: Add SwapMarkerChangedError and the SwapCancelOutcome type to
the public exports in lib/actions/embedder-swap/index.ts, sourcing each from the
module that defines it alongside the existing swap errors and related types.
Preserve all current exports so consumers can name cancelStorySwap’s return type
and catch the phase-2 preflight error through the barrel.

In `@lib/actions/index.ts`:
- Around line 2-29: Remove the raw engine primitives startSwap, resumeSwap,
reindexStory, cancelSwap, relabelModel, and the SwapDeps/SwapParams types from
the lib/actions barrel export. Keep the *Story* wrapper entry points and
error/configuration exports available, leaving primitives reachable only through
the embedder-swap module.

In `@lib/db/runtime/exec.native.ts`:
- Around line 20-31: Update queryRows to avoid getAllSync and unbounded
full-result materialization by using the available asynchronous or chunked
expo-sqlite query mechanism. Preserve its Promise-based API and positional-row
contract by converting each fetched row with Object.values() in SELECT column
order, while allowing callers to process stale rows incrementally.

In `@lib/db/stories/settings-ops.ts`:
- Around line 73-75: Add a dedicated test for the exported setSwapTargetDimOp
function in settings-ops.test.ts, verifying it produces the expected settings
patch for embedding_swap_target_dim and preserves the provided storyId and nowMs
values, consistent with the other settings operation tests.

In `@locales/en/wizard.json`:
- Around line 53-57: Update the storagePreview and footer locale strings to
replace the hardcoded 30 chapter count with the {{chapters}} interpolation, and
replace the abbreviated “ch” with translatable wording. Ensure the corresponding
storage-estimate rendering passes the estimation constant as chapters so both
messages remain synchronized.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 607ad4a5-043a-43ec-bd6e-1a0a3c517f1e

📥 Commits

Reviewing files that changed from the base of the PR and between e488866 and 7cf2142.

⛔ Files ignored due to path filters (9)
  • docs/data-model.md is excluded by !docs/**
  • docs/implementation/milestones/03-memory-floor/slices/01b-embedder-lifecycle.md is excluded by !docs/**
  • docs/implementation/milestones/03-memory-floor/slices/04-retrieval.md is excluded by !docs/**
  • docs/implementation/triage.md is excluded by !docs/**
  • docs/memory/model-management.md is excluded by !docs/**
  • docs/memory/retrieval.md is excluded by !docs/**
  • docs/testing.md is excluded by !docs/**
  • docs/ui/patterns/generation-status-pill.md is excluded by !docs/**
  • docs/ui/screens/reader-composer/reader-composer.md is excluded by !docs/**
📒 Files selected for processing (95)
  • .remarkignore
  • app/_layout.tsx
  • app/reader-composer/[branchId].tsx
  • app/story-settings/[storyId].tsx
  • app/wizard.tsx
  • components/compounds/embedder-default-card.stories.tsx
  • components/compounds/embedder-default-card.tsx
  • components/compounds/generation-status-pill.stories.tsx
  • components/compounds/generation-status-pill.tsx
  • components/embedder/reindex-confirm-dialog.tsx
  • components/embedder/swap-dialog.stories.tsx
  • components/embedder/swap-dialog.tsx
  • components/embedder/swap-resume-dialog.stories.tsx
  • components/embedder/swap-resume-dialog.tsx
  • components/embedder/swap-resume-host.tsx
  • components/story-settings/memory-panel.stories.tsx
  • components/story-settings/memory-panel.tsx
  • components/wizard/finish.test.ts
  • components/wizard/finish.ts
  • components/wizard/memory-cost-disclosure.stories.tsx
  • components/wizard/memory-cost-disclosure.tsx
  • components/wizard/memory-cost-logic.test.ts
  • components/wizard/memory-cost-logic.ts
  • components/wizard/step-opening.tsx
  • e2e/harness/embedder.ts
  • e2e/harness/seed.ts
  • e2e/locators/reader.ts
  • e2e/locators/story-settings.ts
  • e2e/tests/embedder-drain.spec.ts
  • e2e/tests/embedder-swap.spec.ts
  • lib/actions/embedder-swap/app-deps.test.ts
  • lib/actions/embedder-swap/app-deps.ts
  • lib/actions/embedder-swap/engine.test.ts
  • lib/actions/embedder-swap/engine.ts
  • lib/actions/embedder-swap/index.ts
  • lib/actions/index.ts
  • lib/actions/settings/embedder.test.ts
  • lib/actions/settings/embedder.ts
  • lib/actions/settings/index.ts
  • lib/actions/settings/providers.ts
  • lib/actions/stories/operational.ts
  • lib/ai/embedding.test.ts
  • lib/ai/embedding.ts
  • lib/ai/model-capabilities.ts
  • lib/boot/bootstrap.ts
  • lib/db/app-settings/app-settings-schema.test.ts
  • lib/db/app-settings/app-settings-schema.ts
  • lib/db/embeddings/field-rows.test.ts
  • lib/db/embeddings/field-rows.ts
  • lib/db/embeddings/index.ts
  • lib/db/embeddings/ops.test.ts
  • lib/db/embeddings/ops.ts
  • lib/db/embeddings/stale.test.ts
  • lib/db/embeddings/stale.ts
  • lib/db/embeddings/vec-tables.test.ts
  • lib/db/embeddings/vec-tables.ts
  • lib/db/entities/entities.table.ts
  • lib/db/happenings/happenings.table.ts
  • lib/db/index.ts
  • lib/db/lore/lore.table.ts
  • lib/db/migrations/0006_absent_natasha_romanoff.sql
  • lib/db/migrations/meta/0006_snapshot.json
  • lib/db/migrations/meta/_journal.json
  • lib/db/migrations/migrations.js
  • lib/db/runtime/exec.native.ts
  • lib/db/runtime/exec.ts
  • lib/db/stories/settings-ops.test.ts
  • lib/db/stories/settings-ops.ts
  • lib/db/stories/story-config-schema.test.ts
  • lib/db/stories/story-config-schema.ts
  • lib/db/stories/story-settings-defaults.test.ts
  • lib/db/stories/story-settings-defaults.ts
  • lib/db/story-entries/story-entries.table.ts
  • lib/db/threads/threads.table.ts
  • lib/db/wizard-sessions/working-state.ts
  • lib/embedder/drain.test.ts
  • lib/embedder/drain.ts
  • lib/embedder/gate.test.ts
  • lib/embedder/index.ts
  • lib/embedder/resolve-config.test.ts
  • lib/embedder/resolve-config.ts
  • lib/embedder/service.test.ts
  • lib/embedder/service.ts
  • lib/embedder/types.ts
  • lib/stores/generation/generation.ts
  • lib/stores/index.ts
  • lib/stores/ui/embedder-swap.test.ts
  • lib/stores/ui/embedder-swap.ts
  • lib/stores/ui/embedding-status.test.ts
  • lib/stores/ui/embedding-status.ts
  • lib/stores/wizard/wizard.test.ts
  • lib/stores/wizard/wizard.ts
  • locales/en/common.json
  • locales/en/story-settings.json
  • locales/en/wizard.json

Comment thread components/compounds/embedder-default-card.tsx
Comment thread components/wizard/finish.ts
Comment thread lib/actions/embedder-swap/app-deps.test.ts
Comment thread lib/actions/settings/providers.ts
Comment thread lib/db/stories/settings-ops.test.ts Outdated
Comment thread lib/db/stories/settings-ops.ts
Comment thread lib/embedder/drain.ts
failerko and others added 8 commits July 29, 2026 02:13
noteIdle scheduled at 0 through a schedule() that unconditionally cleared the pending timer, so any run_complete replaced an armed 5s/30s/120s retry with an immediate drain — a story generating faster than the ladder retried a down embedder at the idle cadence, which is the cadence the ladder exists to avoid. The guard is story-scoped rather than a bare timer==null: a retry armed for a story the user has navigated away from must not stall the story they are now in.

Four isolation tests drove pass 2 through the clobbering path and now advance the real retry instead; their describe needed fake timers to do it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The in-flight probe dedup is keyed by providerId:modelId, so two probes for different models on one provider run concurrently — and both read cachedModels off the store before either updateProvider rehydrates, so the later write persists a snapshot missing the earlier dim. The new test reproduces the loss without the lock (m-a's dim reads undefined). Scoped to recordProviderEmbeddingDim: an updateProvider from the provider setup form still races it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolves the slice's "custom effective dim has no upper bound" open question: the 2026-07-28 probe followup made capabilities.embeddingDim available at wizard time, dissolving the "the bound cannot be known here" premise the deferral rested on. validateCustomDim now rejects above the ceiling; Finish CLAMPS to null (native) rather than rejecting, because the disclosure is collapsed by default and a stale working-state pick would otherwise fail a field nobody can see — and null also suppresses a serverSide dimensions hint the provider would reject. An unprobed model stays permissive; that residue is M7's upper-bound UI.

Storage-preview copy takes the projected length as {{chapters}} off one constant instead of baking 30 into two strings behind an untranslatable "ch".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Provider ids are generated prov_<uuid> today, so nothing collides — but the schema is a bare z.string() and the docblock promises a stable identity, so the id is encoded rather than trusted. Only the provider id: model ids are free-form and colon-bearing ones are common, but the model id is the trailing segment and cannot make a key parse two ways. encodeURIComponent is the identity for every real provider id, which keeps the key readable as the swap-candidate testID that stories and the E2E locator resolve against.

Two tests were titled after a setEmbeddingModelIdOp that no longer exists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The assertion passed only because its own describe clears mock history and two tests precede it — a reordering away from breaking. lastCall carries no such dependency.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
startSwap / resumeSwap / cancelSwap / reindexStory / relabelModel each require the caller to already hold the per-story single-flight lock (engine.ts header contract), and re-exporting them beside the *Story* wrappers made the unsafe pair the more obvious import with orphaned staged vectors as the failure mode. Nothing outside lib/actions/embedder-swap imported them. SwapMarkerChangedError and the SwapCancelOutcome type go the other way: both cross the boundary — one thrown by the phase-2 preflight, one returned by cancelStorySwap — and were unnameable from outside the folder.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MemoryPanel and SwapResumeHost each carried a byte-equivalent error reporter and the same resume/cancel orchestration down to the already-completed toast, differing only in the pre-step each runs first. The hook owns the engine calls and the reporting contract; the dialog close and the navigate stay at their call sites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@failerko

Copy link
Copy Markdown
Collaborator Author

Nitpick disposition (the six in the review body have no inline threads):

Taken

  • lib/actions/embedder-swap/index.tsSwapMarkerChangedError + SwapCancelOutcome exported (3ce9cf8).
  • lib/actions/index.ts — raw engine primitives dropped from the app-wide barrel (3ce9cf8). Nothing outside lib/actions/embedder-swap imported them; the engine tests import from ./engine intra-module.
  • settings-ops.tssetSwapTargetDimOp test added (b9cb482).
  • locales/en/wizard.json — the untranslatable ch is gone and both strings take {{chapters}} off one exported constant (111710f). Note PROJECTED_ROWS_30CH was a ROW count (2250), not a chapter count, so this needed a separate PROJECTED_CHAPTERS; the rows constant is now PROJECTED_ROWS so its name no longer embeds a number it does not hold.
  • memory-panel.tsx / swap-resume-host.tsx — shared hook extracted (b0066a3). The error reporter was byte-equivalent between the two and the resume/cancel cores matched down to the already-completed toast; only the pre-steps differed, and those stay at the call sites.

Not taken

  • exec.native.ts queryRows. Splitting the finding: getAllSyncgetAllAsync is a one-line change that moves the work off the JS thread and is the real win (the symptom is a frozen Android UI during a re-index, not slow indexing). Incremental/chunked loading is the genuinely heavy part — the swap engine loads the full row set up front and batches embeds at 16, so making the LOAD incremental changes the queryAll contract across engine.ts, stale.ts and field-rows.ts. Holding both: exec.native.ts is Android-only and Vitest exercises the node-sqlite path, so nothing in this repo would verify either change, and I am not shipping an unverifiable edit to a driver seam. Desktop goes through exec.ts and is unaffected.

Full suite green: 2265 tests, typecheck, ESLint, remark.

@failerko
failerko merged commit 3b23d33 into main Jul 29, 2026
4 checks passed
@failerko
failerko deleted the slice/m3.1b-embedder-lifecycle branch July 29, 2026 00:46
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