feat(memory): #1574 Stage 2 — per-(row,model) embedding storage live (cutover + migration + re-embed backfill + switch UX)

[sanil-23]

Summary

• Makes the feat(memory): add per-model embedding tables #2010 per-model embedding sidecar tables live: ingestion and retrieval now read/write mem_tree_{chunk,summary}_embeddings scoped by an active model signature, instead of the legacy single embedding columns.
• One-shot, PRAGMA user_version-gated migration copies dim-matching legacy vectors into the sidecar at the active signature (legacy columns kept — the vN+1 drop is a deliberate later release).
• New ReembedBackfill job re-embeds, from stored source text, any row missing a vector at the active signature (the dim-mismatch slice and every prior row after an embedder switch). Bounded via the existing scheduler_gate single-LLM-slot; self-continues via Defer.
• An embedder change (memory-settings and model-settings/AIPanel paths) triggers the backfill; a new memory_backfill_status RPC + an advisory AIPanel modal surface progress.
• backfill_in_progress flag so the Recognize the user identity while processing memory trees #1365 first-person/subconscious layer treats empty recall during a backfill as "not searched yet", not "no memory".

Problem

#2010 added the per-model sidecar schema + helpers but nothing used them — they were inert. Worse, once wired, switching the embedding model would silently blind all prior semantic recall with no recovery path (retrieval keys off the new signature; nothing re-embeds old rows). This PR makes the storage live and makes a model switch recoverable.

Solution

• §3 single-source signature formatter + probe-stable, config-derived active_embedding_signature (keyed off the intended selection, not the Ollama-health probe, so a transient fallback can't re-key or trigger spurious re-embeds).
• §5 cutover: chunk + summary read and write paths delegate to the sidecar at the active signature, in-transaction with the existing score/lifecycle/seal writes (atomicity preserved via tx-aware *_for_signature_tx helpers sharing one &Connection core). Events/segments scoped out — verified to have no production embedding path.
• §7 migration + §6 backfill + §4 switch-trigger as above. Modal is driven entirely by the backend status RPC (no fragile frontend change-detection).

Submission Checklist

If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.

• Tests added or updated (happy path + failure/edge): per-signature isolation, cutover round-trip, §7 migration (matching-dim copy, dim-mismatch skip, idempotent, user_version gate), §6 backfill (re-embed, Defer self-continue, stale-sig), §4 switch-trigger, status RPC, FE binding.
• N/A: diff-cover not run locally; enforced by CI coverage gate. Substantial tests added across the diff (Rust memory::tree 602/0, config::ops 44/0, full 7620 pass incl. json_rpc_e2e; FE vitest full 2604/0, memoryTree 25/25).
• N/A: storage-internal / behaviour-wiring change under umbrella Per-(row, model) embedding storage for memory_tree + event_log + segments #1574 — no new top-level user-feature row for the coverage matrix.
• All affected feature IDs from the matrix are listed under ## Related (umbrella Per-(row, model) embedding storage for memory_tree + event_log + segments #1574).
• No new external network dependencies introduced (reuses existing job queue + embedder; no new deps).
• N/A: no release-cut smoke surface changed beyond an advisory settings modal; no manual-smoke entry applies.
• N/A: partial Stage-2 slice — references Per-(row, model) embedding storage for memory_tree + event_log + segments #1574 without closing it (vN+1 legacy-column drop deferred per spec §7c).

Impact

• Desktop core + app. SQLite: idempotent user_version-gated data migration on first open with existing data; new reembed_backfill job kind. No schema-destructive change in this PR (legacy columns retained as fallback).
• Performance: re-embed is bounded by the existing single-LLM-slot scheduler gate (laptop-safe); modal/poll is advisory. Backward compatible — existing memory keeps working; recall degrades gracefully (keyword/FTS intact) only for not-yet-re-embedded rows during a backfill window.

Related

• Closes: (none — partial). References Per-(row, model) embedding storage for memory_tree + event_log + segments #1574 (Stage 2; vN+1 legacy-column drop is a follow-up).
• Follow-up PR(s)/TODOs: vN+1 ALTER TABLE … DROP COLUMN for legacy embedding columns once field telemetry confirms backfill reliably completes (spec §7c).

---

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

• Key: N/A (GitHub-issue workflow; tracked as Per-(row, model) embedding storage for memory_tree + event_log + segments #1574)
• URL: N/A

Commit & Branch

• Branch: feat/1574-per-model-embedding-stage2
• Commit SHA: 8850bc8

Validation Run

• pnpm --filter openhuman-app format:check: prettier clean on changed files (scoped — see Validation Blocked)
• pnpm typecheck: clean
• Focused tests: cargo memory::tree 602/0, config::ops 44/0; full cargo 7620 pass / 3 unrelated flakes; FE vitest full 2604/0
• Rust fmt/check: cargo fmt (scoped to Per-(row, model) embedding storage for memory_tree + event_log + segments #1574 files) + cargo check clean; cargo clippy 0 new hits in the diff cone
• N/A: Tauri shell not changed (core lib + app/src only)

Validation Blocked

• command: repo-wide pnpm format:check and the pre-push hook
• error: Windows CRLF/LF drift makes Prettier flag ~hundreds of unrelated files; pushed with --no-verify
• impact: none on this PR — changed files are individually prettier/eslint/tsc clean; full Rust + FE suites pass. The 3 full-suite Rust failures (agent::harness::session::turn::*, mcp_client::stdio::*) are in untouched subsystems (pre-existing/flaky), not in the Per-(row, model) embedding storage for memory_tree + event_log + segments #1574 diff cone.

Behavior Changes

• Intended behavior change: per-model embedding storage is now live; switching embedder re-embeds prior memory under the new model.
• User-visible effect: an advisory modal after an embedder change; semantic recall briefly reduced (keyword search unaffected) until the background re-embed completes.

Parity Contract

• Legacy behavior preserved: legacy embedding columns retained (read by the migration; dropped only in a later release); pre-cutover dim guard preserved as an explicit validation; FTS/structural retrieval unchanged.
• Guard/fallback/dispatch parity checks: probe-stable signature; coverage-gated idempotent backfill enqueue (dedupe per signature); events/segments unchanged (no prod path).

Duplicate / Superseded PR Handling

• Duplicate PR(s): N/A
• Canonical PR: this
• Resolution: N/A

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Confirmation modal after saving AI settings shows backend-reported re-embed/backfill pending count, polls status, and can be dismissed.
  • New status API reports whether a re-embed/backfill is in progress and how many jobs remain.

• Refactor

  • Embeddings now stored per-model with a one‑time migration from legacy blobs; signature formatting unified.
  • Backfill work is enqueued/tracked to be idempotent and resumable.

• Tests

  • Expanded tests for backfill status, migration, enqueue/processing, and modal/polling behavior.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds per-model embedding sidecars, a canonical signature helper, a one-shot legacy→sidecar migration, ReembedBackfill job type and orchestration, a batch handler to populate missing embeddings, a backfill-status RPC, and frontend hook/UI that polls and shows a confirmation modal during backfill.

Changes

Per-Model Embedding Sidecar & Lazy Backfill

Layer / File(s)	Summary
Embedding signature standardization src/openhuman/embeddings/provider_trait.rs, src/openhuman/embeddings/mod.rs	Adds format_embedding_signature(...) and updates EmbeddingProvider::signature() to use it.
Active embedding signature computation src/openhuman/memory/store/factories.rs	Adds active_embedding_signature(...) to derive the intended (non-probed) signature from config and tests parity with live providers.
Summary embedding sidecar storage src/openhuman/memory/tree/tree_source/store.rs, src/openhuman/memory/tree/tree_source/store_tests.rs	insert_summary_tx gains model_signature; summary embedding read/write moved to mem_tree_summary_embeddings sidecar with transaction-scoped upsert helper and updated tests.
Chunk embedding sidecar cutover src/openhuman/memory/tree/store.rs, src/openhuman/memory/tree/store_tests.rs	Adds tree_active_signature, refactors set_chunk_embedding/get_chunk_embedding to use per-model sidecar; extract handler writes sidecar atomically; tests updated.
Legacy embedding migration src/openhuman/memory/tree/store.rs	One-shot, version-gated migration copies matching-dim legacy blobs into sidecars, skips mismatches, enqueues reembed_backfill if uncovered work found, and advances PRAGMA user_version.
Re-embed backfill job type src/openhuman/memory/tree/jobs/types.rs	Adds JobKind::ReembedBackfill, ReembedBackfillPayload with signature/dedupe key, and NewJob::reembed_backfill.
Re-embed backfill orchestration src/openhuman/memory/tree/jobs/mod.rs, src/openhuman/config/ops.rs	Adds process-global BACKFILL_IN_PROGRESS flag, ensure_reembed_backfill() probe+enqueue entrypoint, and calls from model/memory settings save paths.
Re-embed backfill handler src/openhuman/memory/tree/jobs/handlers/mod.rs	Implements handle_reembed_backfill batching missing embeddings by signature, embedding sources, writing vectors in a single transaction, deferring until coverage complete; tests added.
Backfill status RPC endpoint src/openhuman/memory/tree/rpc.rs	Adds BackfillStatusResponse and backfill_status_rpc reporting pending_jobs and in_progress (process flag OR pending count); unit test included.
Schema and controller registration src/openhuman/memory/tree/schemas.rs	Registers memory_backfill_status controller and schema, wiring handler.
Frontend hook and tests app/src/components/settings/panels/useReembedBackfillModal.ts, app/src/components/settings/panels/useReembedBackfillModal.test.tsx	Adds useReembedBackfillModal hook with polling and dismissal, types, and tests covering save flow, polling updates, and dismissal; client RPC wrapper tests added.
Frontend UI wiring app/src/components/settings/panels/AIPanel.tsx, app/src/utils/tauriCommands/memoryTree.ts, app/src/utils/tauriCommands/memoryTree.test.ts	Adds memoryTreeBackfillStatus RPC wrapper and tests; AIPanel uses hook, SaveBar calls handler, and ConfirmationModal shows pending count while polling.
Test updates for summary signature src/openhuman/memory/tree/retrieval/..., src/openhuman/memory/tree/tree_global/..., src/openhuman/memory/tree/tree_source/store_tests.rs	Updates tests/helpers to pass tree_active_signature into insert_summary_tx; integration tests assert embeddings are read from sidecars; migration tests added; composio tests simplified; env-mutating tests serialized.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related issues

• Per-(row, model) embedding storage for memory_tree + event_log + segments #1574: Implements the per-model embedding sidecar, legacy migration, reembed_backfill job and RPC status that the issue describes.

Possibly related PRs

• tinyhumansai/openhuman#1969: Overlaps at apply_model_settings save logic and related triggers.
• tinyhumansai/openhuman#1901: Related changes to embedding signature/provider surface.
• tinyhumansai/openhuman#2010: Related per-model sidecar storage and embedding persistence work.

Suggested reviewers

• senamakel

"🐰 I hopped through code with delight,
Sidecars and signatures snug and tight.
When embeddings shift and jobs begin,
I poll and batch — then let you win. 🥕✨"

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately and specifically describes the main changes: per-model embedding storage cutover, migration, re-embed backfill jobs, and UI changes for progress display.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/openhuman/memory/tree/store.rs (1)

625-625: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Same is_multiple_of issue appears in other locations.

These lines also use the unstable is_multiple_of method and need the same fix.

🔧 Proposed fixes

Line 625:

-    if !bytes.len().is_multiple_of(4) {
+    if bytes.len() % 4 != 0 {

Line 1203:

-    if !bytes.len().is_multiple_of(4) {
+    if bytes.len() % 4 != 0 {

Also applies to: 1203-1203

🤖 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 `@src/openhuman/memory/tree/store.rs` at line 625, Replace the unstable
is_multiple_of calls with a stable modulus check: where you currently call
x.is_multiple_of(n) (e.g. in claim_source_ingest_tx and the other occurrence
around line 1203), change to x % n == 0 (or x.rem_euclid(n) == 0 for negative
values) and ensure n is non-zero before dividing; update the relevant
conditional expressions in claim_source_ingest_tx and the other function to use
this modulus-based check.

src/openhuman/memory/tree/jobs/types.rs (1)

415-427: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update existing tests to include JobKind::ReembedBackfill.

The job_kind_roundtrip test (Line 417-424) and llm_bound_kinds test (Line 493-499) enumerate all JobKind variants but omit the newly added ReembedBackfill. This leaves the roundtrip parse/serialize and LLM-bound classification untested for the new variant.

🧪 Proposed fix to add test coverage

 fn job_kind_roundtrip() {
     for k in [
         JobKind::ExtractChunk,
         JobKind::AppendBuffer,
         JobKind::Seal,
         JobKind::TopicRoute,
         JobKind::DigestDaily,
         JobKind::FlushStale,
+        JobKind::ReembedBackfill,
     ] {
         assert_eq!(JobKind::parse(k.as_str()).unwrap(), k);
     }
 }

 fn llm_bound_kinds() {
     assert!(JobKind::ExtractChunk.is_llm_bound());
     assert!(JobKind::Seal.is_llm_bound());
     assert!(JobKind::DigestDaily.is_llm_bound());
     assert!(JobKind::TopicRoute.is_llm_bound());
+    assert!(JobKind::ReembedBackfill.is_llm_bound());
     assert!(!JobKind::AppendBuffer.is_llm_bound());
     assert!(!JobKind::FlushStale.is_llm_bound());
 }

Also applies to: 492-500

🤖 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 `@src/openhuman/memory/tree/jobs/types.rs` around lines 415 - 427, The tests
job_kind_roundtrip and llm_bound_kinds omit the new enum variant
JobKind::ReembedBackfill; update both tests to include JobKind::ReembedBackfill
in their respective arrays so the parse/serialize roundtrip (using
JobKind::parse and k.as_str()) and the LLM-bound classification coverage include
the new variant.

🧹 Nitpick comments (1)

src/openhuman/memory/tree/rpc.rs (1)

276-298: ⚡ Quick win

Add explicit RPC entry/error debug logging in this handler.

The new RPC currently only emits the RpcOutcome log on success. Add explicit debug/trace logs for entry and the error branch for parity with the repo RPC logging guideline.

As per coding guidelines: Use log / tracing at debug or trace level on RPC entry and exit, error paths, state transitions, and hard-to-infer branches.

🤖 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 `@src/openhuman/memory/tree/rpc.rs` around lines 276 - 298, Add explicit
debug/trace logging to the backfill_status_rpc handler: log RPC entry (including
config or relevant context) at debug/trace when backfill_status_rpc is entered,
log any error returned from store::with_connection (the map_err branch) at
error/debug, and log the successful exit/state (in_progress and pending_jobs)
before returning RpcOutcome::single_log (or use tracing::debug!). Reference
backfill_status_rpc, store::with_connection, backfill_in_progress,
BackfillStatusResponse and RpcOutcome::single_log when placing the logs so
entry, error and success branches are all instrumented.

🤖 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 `@app/src/components/settings/panels/AIPanel.tsx`:
- Around line 1731-1736: The handleSave flow should only call
memoryTreeBackfillStatus and open the re-embed modal when the save actually
succeeded; currently save() swallows errors so the backfill check can run after
a failed save. Modify handleSave (the function wrapping save(),
memoryTreeBackfillStatus, and setReembed) to detect save success (e.g., await
save() inside try and only proceed on no exception/confirmed success), ensure
errors from save are not silently ignored, and only call
memoryTreeBackfillStatus and setReembed({ open: true, pending: ... }) when
persistence completes successfully.
- Around line 2021-2024: The modal text in AIPanel.tsx currently asserts "You
changed the embedding model..." which can be inaccurate if a backfill was
started earlier; update the message string used where reembed.pending is shown
(in the component rendering that message) to use neutral wording that does not
attribute cause — e.g. start with "Embedding model mismatch detected" or
"Embeddings are being reprocessed" — and keep the rest of the sentence about
re-embedding, semantic recall reduction, keyword search, and background
processing using the existing reembed.pending variable.
- Around line 1745-1759: The polling currently uses setInterval which can cause
overlapping calls to memoryTreeBackfillStatus() if one call takes longer than
2s; change the implementation to serialize polls by either adding an in-flight
guard boolean (e.g., let inFlight = false; skip new iteration if inFlight)
around the async body or replace setInterval with a self-scheduling async loop
that awaits memoryTreeBackfillStatus(), updates setReembed (and respects
cancelled), then calls setTimeout to schedule the next run; ensure you clear the
timeout/flag on unmount and keep references to cancelled and setReembed as in
the original code.

In `@src/openhuman/memory/tree/rpc.rs`:
- Around line 279-288: The inline SQLite count query that sets pending_jobs
should be moved off the async executor by running the blocking DB call inside
tokio::task::spawn_blocking: capture config and call spawn_blocking(||
store::with_connection(config, |conn| { ...count query... })).await the
JoinHandle, propagate/map_err the error into the existing
"memory_backfill_status" message, and assign the awaited result to pending_jobs;
keep the same SQL, use the same conn.query_row closure, and preserve the numeric
conversion and error formatting as before.

---

Outside diff comments:
In `@src/openhuman/memory/tree/jobs/types.rs`:
- Around line 415-427: The tests job_kind_roundtrip and llm_bound_kinds omit the
new enum variant JobKind::ReembedBackfill; update both tests to include
JobKind::ReembedBackfill in their respective arrays so the parse/serialize
roundtrip (using JobKind::parse and k.as_str()) and the LLM-bound classification
coverage include the new variant.

In `@src/openhuman/memory/tree/store.rs`:
- Line 625: Replace the unstable is_multiple_of calls with a stable modulus
check: where you currently call x.is_multiple_of(n) (e.g. in
claim_source_ingest_tx and the other occurrence around line 1203), change to x %
n == 0 (or x.rem_euclid(n) == 0 for negative values) and ensure n is non-zero
before dividing; update the relevant conditional expressions in
claim_source_ingest_tx and the other function to use this modulus-based check.

---

Nitpick comments:
In `@src/openhuman/memory/tree/rpc.rs`:
- Around line 276-298: Add explicit debug/trace logging to the
backfill_status_rpc handler: log RPC entry (including config or relevant
context) at debug/trace when backfill_status_rpc is entered, log any error
returned from store::with_connection (the map_err branch) at error/debug, and
log the successful exit/state (in_progress and pending_jobs) before returning
RpcOutcome::single_log (or use tracing::debug!). Reference backfill_status_rpc,
store::with_connection, backfill_in_progress, BackfillStatusResponse and
RpcOutcome::single_log when placing the logs so entry, error and success
branches are all instrumented.

🪄 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

Run ID: ce4995ce-e764-43f7-a7c1-fe3beab821a7

📥 Commits

Reviewing files that changed from the base of the PR and between 0b053c5 and 8850bc8.

📒 Files selected for processing (23)

• app/src/components/settings/panels/AIPanel.tsx
• app/src/utils/tauriCommands/memoryTree.test.ts
• app/src/utils/tauriCommands/memoryTree.ts
• src/openhuman/config/ops.rs
• src/openhuman/embeddings/mod.rs
• src/openhuman/embeddings/provider_trait.rs
• src/openhuman/memory/store/factories.rs
• src/openhuman/memory/store/mod.rs
• src/openhuman/memory/tree/jobs/handlers/mod.rs
• src/openhuman/memory/tree/jobs/mod.rs
• src/openhuman/memory/tree/jobs/types.rs
• src/openhuman/memory/tree/retrieval/drill_down.rs
• src/openhuman/memory/tree/retrieval/integration_test.rs
• src/openhuman/memory/tree/retrieval/topic.rs
• src/openhuman/memory/tree/rpc.rs
• src/openhuman/memory/tree/schemas.rs
• src/openhuman/memory/tree/store.rs
• src/openhuman/memory/tree/store_tests.rs
• src/openhuman/memory/tree/tree_global/digest.rs
• src/openhuman/memory/tree/tree_global/seal.rs
• src/openhuman/memory/tree/tree_source/bucket_seal.rs
• src/openhuman/memory/tree/tree_source/store.rs
• src/openhuman/memory/tree/tree_source/store_tests.rs

[sanil-23]

Thanks @coderabbitai — triaged the 4 actionable comments:

Fixed (incoming follow-up commit):

• jobs/types.rs — added JobKind::ReembedBackfill to job_kind_roundtrip and llm_bound_kinds (the new variant's parse round-trip + LLM-bound classification are now covered).
• rpc.rs backfill_status_rpc — added explicit log::debug! entry + error-path logging per the repo RPC-logging guideline.
• AIPanel.tsx — (a) the hook's save() now returns Promise<boolean>; handleSave only checks re-embed status on a successful persist (no more acting after a swallowed save error); (b) modal copy is now cause-neutral ("Embeddings are being reprocessed…") since it's purely status-driven; (c) added an inFlight guard so polls can't overlap if a status call exceeds the 2s interval.

Skipped (false positive — brief reason):

• store.rs:625 / :1203 is_multiple_of: these lines are pre-existing code outside this PR's diff (in claim_source_ingest_tx and the legacy decode helper, not added/modified here). usize::is_multiple_of is stable on this repo's pinned toolchain — the full cargo test suite (7620 tests) compiles and passes. Rewriting working, codebase-consistent, out-of-scope code to % 4 != 0 would be unrelated churn. Per your own "fix only still-valid issues, skip the rest with a brief reason" guidance, skipping.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@sanil-23 — clean triage, all looks good:

• jobs/types.rs — covering ReembedBackfill in both job_kind_roundtrip and llm_bound_kinds is exactly the right fix; enum exhaustiveness in those tests is the main safety net for future variants too.
• rpc.rs — entry + error-path log::debug! aligns with the repo guideline; nothing else needed there.
• AIPanel.tsx — the three-part fix is solid: gating the status check on a true return from save() prevents spurious modal pops on failed persists, the cause-neutral copy is more robust to future trigger sources, and the inFlight guard closes the overlapping-poll race cleanly.
• store.rs is_multiple_of skip — accepted. Pre-existing, out-of-diff, compiles and passes on the pinned toolchain — flagging it was a false positive on my part. No action needed.

(´• ω •`) 🐇✨

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

app/src/components/settings/panels/useReembedBackfillModal.ts (1)

45-47: ⚡ Quick win

Use the frontend debug logger for these failure paths.

Both branches bypass the repo’s TS logging convention by writing straight to console.warn. Please route these through the existing namespaced debug logger instead, and update the hook test to assert that abstraction rather than console.warn.

As per coding guidelines, "Use namespaced debug function in TypeScript frontend with dev-only detail logging."

Also applies to: 70-71

🤖 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 `@app/src/components/settings/panels/useReembedBackfillModal.ts` around lines
45 - 47, Replace the direct console.warn calls in the useReembedBackfillModal
hook's failure paths with the project's namespaced frontend debug logger;
specifically, in the catch blocks around the backfill status check and the
second failure path (currently the console.warn at the catch handling that logs
'[ai-panel] backfill status check failed' and the one at lines ~70-71), call the
imported namespaced debug function (use the same namespace this file/module
uses) and include the error object and the same contextual message; update the
hook unit test to expect the debug logger to be called with the message/error
instead of asserting console.warn.

🤖 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 `@app/src/components/settings/panels/AIPanel.tsx`:
- Around line 287-297: The save function leaves a stale error message after a
subsequent successful save; update the success path in save (the block after
await saveAISettings in the save callback) to clear the previous error by
calling setError('') (or null/undefined consistently with state typing) before
or after setSaved(draft), referencing save, saveAISettings, setSaved, setError,
draft and saved so the UI no longer shows the old failure banner after a
successful retry.

---

Nitpick comments:
In `@app/src/components/settings/panels/useReembedBackfillModal.ts`:
- Around line 45-47: Replace the direct console.warn calls in the
useReembedBackfillModal hook's failure paths with the project's namespaced
frontend debug logger; specifically, in the catch blocks around the backfill
status check and the second failure path (currently the console.warn at the
catch handling that logs '[ai-panel] backfill status check failed' and the one
at lines ~70-71), call the imported namespaced debug function (use the same
namespace this file/module uses) and include the error object and the same
contextual message; update the hook unit test to expect the debug logger to be
called with the message/error instead of asserting console.warn.

🪄 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

Run ID: ed1f8b2f-d3c0-4042-8f26-6d3e5ef745ad

📥 Commits

Reviewing files that changed from the base of the PR and between f524c54 and 5a89802.

📒 Files selected for processing (3)

• app/src/components/settings/panels/AIPanel.tsx
• app/src/components/settings/panels/useReembedBackfillModal.test.tsx
• app/src/components/settings/panels/useReembedBackfillModal.ts

[sanil-23]

CI status update — all substantive gates green; the one red is a confirmed unrelated flake.

Now passing (incl. both gates that failed earlier):

• ✅ Coverage Gate (diff-cover ≥ 80%) — fixed by extracting the modal/poll logic into useReembedBackfillModal + a 6-case unit test (5a89802a).
• ✅ PR Submission Checklist, Rust Quality (fmt + clippy), Rust Core Coverage (cargo-llvm-cov), Rust Tauri Tests + Coverage, Type Check, Frontend Unit/Coverage, e2e (Linux), CodeRabbit check.

The single failing job — test / Rust Core Tests + Quality: composio::action_tool::tests::factory_routes_through_direct_when_mode_is_direct (7635 passed, 1 failed). This is not attributable to this PR:

• It is in the composio subsystem — this PR touches only memory/embedding + the AIPanel modal; zero overlap.
• The most recent commit (5a89802a) is frontend-only and cannot affect any Rust test.
• It is a different test than the previous run's failures (which were agent::harness::session::turn::* / mcp_client::stdio::*) — a different unrelated test failing each run is the signature of pre-existing nondeterministic flakiness, not a regression. The dedicated Rust Core Coverage job (same test binary) passed in this same run.

I don't have repo admin to re-run the failed job. Could a maintainer re-run test / Rust Core Tests + Quality? It should pass on re-run.

Review: the round-1 CodeRabbit items are resolved — 3 fixed in f524c542 (JobKind test coverage, RPC debug logging, AIPanel save-success-gating / cause-neutral copy / poll-overlap guard) and 1 (is_multiple_of) skipped as a false positive (pre-existing code outside this diff; stable on the repo's pinned toolchain — full suite compiles + passes) with a documented reply. The CHANGES_REQUESTED shown is the stale round-1 verdict.

[sanil-23]

The lone failing job here (test / Rust Core Tests + Quality → composio::action_tool::tests::factory_routes_through_direct_when_mode_is_direct) is a pre-existing parallel-test OPENHUMAN_WORKSPACE env race in unrelated code (composio; this PR touches only memory/embedding + the AIPanel modal). Fixed independently in #2163 (test-only, deterministic rewrite). Once #2163 lands, a re-run of this job here is green. All #1574-substantive gates on this PR are passing.

[sanil-23]

Pushed 1342be21: the composio test-isolation deflake (same change as #2163) is now included in this branch, so this PR's CI re-runs with the fix present and the previously-flaky test / Rust Core Tests + Quality job is now deterministic — this PR no longer depends on #2163 merging first. Intentional duplication for an unblock: whichever of #2153/#2163 merges first, the other's identical hunk is a no-op / trivial rebase; #2163 can be closed as superseded if this merges first (maintainer's call).