fix(memory_tree,scripts): IMMEDIATE-tx ingest + reembed skip-persistence + Windows dev-script PATH fixes#2349
Conversation
…istence + Windows dev-script PATH fixes
memory_tree::ingest::persist() now opens an IMMEDIATE-tx for the per-message
read-then-write block. Under WAL with multiple writers (4 workers + scheduler
+ ingest), a DEFERRED tx takes a read lock then tries to upgrade to write —
and SQLite returns SQLITE_BUSY *immediately* for that upgrade, bypassing the
connection's 15s busy_timeout (deadlock-avoidance behaviour, documented).
Before this fix every Gmail per-message ingest_email failed with 'database
is locked', dropping all messages and stalling composio_sync. IMMEDIATE
acquires the write lock at BEGIN where busy_timeout DOES apply; writers
serialise and wait instead of failing fast. Live evidence: 13/13 fails -> 0/N.
memory_tree::jobs reembed_backfill now persistently tombstones chunks that
fail terminally (body file missing on disk, embed wrong-dim, embed err)
so the worklist query stops re-selecting them on every batch. Two new
idempotent CREATE TABLE IF NOT EXISTS rows: mem_tree_chunk_reembed_skipped
and mem_tree_summary_reembed_skipped. Two new writer helpers
(chunk_store::mark_chunk_reembed_skipped, tree_source::store::mark_summary_
reembed_skipped). Three worklist/probe SQL queries gain an AND NOT EXISTS
... reembed_skipped clause (handler batch in handlers/mod.rs, the
has_uncovered runtime probe in jobs/mod.rs, the has_uncovered migration
probe in store.rs). The handler's 'skipping' log branches now also write
a sentinel via let _ = mark_*_reembed_skipped(...). Live verification on a
dev workspace: 308 orphan chunks each attempted exactly once, sentinel-
marked, chain reached 'fully covered; chain complete' in ~2 min — replaces
a runaway that previously generated ~128k 'body read failed; skipping'
warns across ~8k batch defers for ~16 stuck chunks because the 'skipping'
was log-only with no persistent exit gate.
scripts/run-dev-win.sh — two Windows-contributor bugfixes for
'pnpm dev:app:win':
(1) After PNPM_EXE is resolved, prepend dirname($PNPM_EXE) to PATH so
Tauri's cmd /S /C 'pnpm run dev' beforeDevCommand can resolve bare
'pnpm' from cmd.exe's PATH. The script otherwise only ever calls
pnpm by absolute path, so its dir was never on PATH and Tauri died
with ''pnpm' is not recognized as an internal or external command'.
(2) Right before '"$PNPM_EXE" tauri dev', deduplicate PATH first-seen-
wins so the MSYS->Windows env-conversion for the spawned native cmd
doesn't overflow the process-env block limit. Without dedup, vcvars
rebuild + Git-Bash /etc/profile re-runs + pnpm .bin layering stack
the system PATH ~5x over (25k+ chars); MSYS conversion drops it and
the child cmd inherits an EMPTY PATH (not 'pnpm' missing — *every*
Windows PATH entry missing including %SystemRoot%\System32, so even
'where' is 'not recognized'). Dedup collapses 317 -> 71 entries on
this box, well under the env-block ceiling, and beforeDevCommand
starts running.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughFixes Windows dev pnpm discovery and PATH overflow, changes ingest.persist() to use an Immediate SQLite transaction, and prevents infinite re-arming of re-embed backfill by adding tombstone tables, helpers, worklist/probe filters, persistent tombstoning on embed errors, and a regression test. ChangesWindows Dev Script & Persistence Configuration
Re-embed Backfill Terminal Failure Handling
Possibly related issues
Possibly Related PRs
Suggested labels
Suggested Reviewers
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Pure formatting. CI Rust Quality (fmt) gate flagged my reembed-skip mark_chunk_reembed_skipped call site for the 'embed wrong dim' branch in handle_reembed_backfill: rustfmt prefers one argument per line for that signature with the call's indentation context. No semantic change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
coderabbitai
Bot
added
the
memory
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/run-dev-win.sh (1)
497-498: 💤 Low valueOptional: Separate declaration and assignment to avoid masking errors.
Static analysis (SC2155) flags that combining
exportwith command substitution masks the exit code ofdirname. Whiledirnameon a validated path is highly reliable, the pattern could be split for defensive consistency.♻️ Proposed fix
-export PATH="$(dirname "$PNPM_EXE"):$PATH" -echo "[run-dev-win] pnpm dir prepended to PATH: $(dirname "$PNPM_EXE")" +PNPM_DIR="$(dirname "$PNPM_EXE")" +export PATH="$PNPM_DIR:$PATH" +echo "[run-dev-win] pnpm dir prepended to PATH: $PNPM_DIR"🤖 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 `@scripts/run-dev-win.sh` around lines 497 - 498, The export line currently combines assignment and command substitution which masks dirname's exit code; split it into two statements: first compute the dirname result into a variable (e.g., pnpm_dir=$(dirname "$PNPM_EXE") and check for errors), then export PATH="$pnpm_dir:$PATH" and echo using that variable. Update the lines around the PATH assignment and the echo in scripts/run-dev-win.sh (references: PNPM_EXE, dirname, PATH, and the echo "[run-dev-win] ...") so the dirname failure won't be masked.
🤖 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.
Nitpick comments:
In `@scripts/run-dev-win.sh`:
- Around line 497-498: The export line currently combines assignment and command
substitution which masks dirname's exit code; split it into two statements:
first compute the dirname result into a variable (e.g., pnpm_dir=$(dirname
"$PNPM_EXE") and check for errors), then export PATH="$pnpm_dir:$PATH" and echo
using that variable. Update the lines around the PATH assignment and the echo in
scripts/run-dev-win.sh (references: PNPM_EXE, dirname, PATH, and the echo
"[run-dev-win] ...") so the dirname failure won't be masked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e64773b6-d1a7-49bb-8ff4-0261e1abc3b7
📒 Files selected for processing (6)
scripts/run-dev-win.shsrc/openhuman/memory/tree/ingest.rssrc/openhuman/memory/tree/jobs/handlers/mod.rssrc/openhuman/memory/tree/jobs/mod.rssrc/openhuman/memory/tree/store.rssrc/openhuman/memory/tree/tree_source/store.rs
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
Review — PR #2349: IMMEDIATE-tx ingest + reembed skip-persistence + Windows PATH fixes
Three real, production-observed bugs fixed here, and the root-cause analysis in the PR description is thorough and accurate. The IMMEDIATE-tx change is the right fix for the SQLite busy-handler bypass, the tombstone table design is clean (FK + ON DELETE CASCADE, idempotent upsert, index on signature), and the Windows PATH dedup logic is correct. Approach approved — one blocker before un-drafting.
CodeRabbit flagged SC2155 on the export PATH=$(dirname ...) line in run-dev-win.sh — skipped here.
[major] Tombstone exclusion path has no test coverage
The tombstone mechanism is the heart of the reembed fix, but no test exercises it. The existing reembed_backfill_repopulates_then_completes (line 1062) covers only the success path — it never triggers the failure branches where mark_chunk_reembed_skipped / mark_summary_reembed_skipped are called, so the critical new invariants are unverified:
- A terminal failure (body file missing, wrong embed dim) actually writes a tombstone row.
- The next batch's worklist
NOT EXISTS … reembed_skippedclause excludes it. has_uncovered(in bothjobs/mod.rsand the migration probe instore.rs) returnsfalsewhen every remaining unembedded chunk/summary has a tombstone — i.e., the chain terminates instead of re-arming forever.
Without (3) in particular, a test regression could silently reintroduce the infinite-rearm loop. A dedicated test that seeds an orphan chunk, runs one backfill batch, asserts the tombstone row was written, runs the has_uncovered probe, and asserts it returns false would close this gap cleanly. The test harness already has reembed_backfill_repopulates_then_completes as a scaffold to follow.
[minor] Fully-qualified crate::... path for summary tombstone calls is asymmetric
See inline comment — chunk side uses the chunk_store:: alias, summary side spells out the full crate::openhuman::memory::tree::tree_source::store:: path. Just add a use import at the top of the handler.
[minor] No built-in path to un-tombstone a row
Once a (chunk_id, model_signature) is tombstoned, the only way to clear it is direct DELETE FROM mem_tree_chunk_reembed_skipped SQL — there's no admin RPC or helper. The PR mentions a follow-up cleanup script for the 308 dev-workspace rows. Worth filing the issue now so it doesn't get lost; if a contributor moves their workspace back the orphan chunks will silently stay skipped until tombstones are cleared manually.
Summary table
| File | Finding | Severity |
|---|---|---|
jobs/handlers/mod.rs |
No test for tombstone write / worklist exclusion / has_uncovered termination |
major |
jobs/handlers/mod.rs |
Summary tombstone calls use fully-qualified crate::... path instead of a use import (chunk side uses chunk_store:: alias) |
minor |
store.rs / tree_source/store.rs |
No un-tombstone helper; manually clear tombstones if workspace paths are repaired | minor |
The code logic itself is solid — no semantic bugs found. This is a test-coverage gate only.
| "[memory_tree::jobs] reembed_backfill: summary {id} embed wrong dim, skipping (sig={active_sig})" | ||
| ); | ||
| let _ = crate::openhuman::memory::tree::tree_source::store::mark_summary_reembed_skipped( | ||
| config, id, &active_sig, "embed wrong dim", |
There was a problem hiding this comment.
[minor] The chunk tombstone calls above use chunk_store::mark_chunk_reembed_skipped(...) (module alias), but the summary side spells out the full crate::openhuman::memory::tree::tree_source::store::mark_summary_reembed_skipped(...) path. This repeats three times in the summary loop and is noisy.
Add a use at the top of the file:
use crate::openhuman::memory::tree::tree_source::store as summary_store;Then call summary_store::mark_summary_reembed_skipped(...) to match the chunk_store:: pattern.
There was a problem hiding this comment.
Done in 09331d8 — added use crate::openhuman::memory::tree::tree_source::store as summary_store; at the top of jobs/handlers/mod.rs alongside the existing chunk_store alias, and converted the three mark_summary_reembed_skipped calls in the reembed loop. Kept the change scoped to the calls this PR introduced — the older tree_source::store:: fully-qualified calls at lines 465 / 486 / 531 / 745 are pre-existing (from PR #2153) and out of scope here.
…as, SC2155 fix Addresses review on tinyhumansai#2349: - **[M3gA-Mind: major]** Add `reembed_backfill_tombstones_orphan_and_terminates` test pinning the §6 invariants: (1) mark_chunk_reembed_skipped writes a row on body-read failure (2) next batch's worklist NOT EXISTS clause excludes the tombstoned row (3) ensure_reembed_backfill's has_uncovered probe reports covered Without (3) a regression could silently reintroduce the infinite-rearm loop (~128 k warns / ~8 k defers per 16 orphans, the original bug). - **[M3gA-Mind: minor]** Switch the three `mark_summary_reembed_skipped` calls in `handle_reembed_backfill` to use a `summary_store::` alias, matching the existing `chunk_store::` pattern in the same file. - **[CodeRabbit: SC2155]** Split `export PATH="$(dirname "$PNPM_EXE"):$PATH"` in `run-dev-win.sh` into a separate `PNPM_DIR=$(dirname …)` assignment + export, so a dirname failure isn't swallowed by the enclosing export. - **[M3gA-Mind: minor]** Un-tombstone helper tracked in tinyhumansai#2358 — left as a follow-up per the review (sticky tombstones are intentional; the manual-operator interface is the right scope). Validation: - cargo test --lib openhuman::memory::tree → 598 passed (including the new test) - cargo fmt --check → clean - cargo clippy -p openhuman --no-deps → no new warnings on touched files Co-Authored-By: Claude <noreply@anthropic.com>
|
Thanks for the thorough review — all three points addressed in 09331d8.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/run-dev-win.sh`:
- Around line 501-503: PNPM_DIR may be "." when PNPM_EXE is not a path-like
value, which would prepend the current working directory to PATH; after
computing PNPM_DIR from PNPM_EXE, guard the export by checking PNPM_DIR is
non-empty and not "." (and optionally starts with "/" or contains a path
separator) before doing export PATH="$PNPM_DIR:$PATH" and echo; update the block
that sets PNPM_DIR and exports PATH (references: PNPM_EXE, PNPM_DIR, PATH) to
perform this conditional check and only prepend when the check passes.
🪄 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: 442a8d46-edae-4716-9dc9-88f993e6bc84
📒 Files selected for processing (2)
scripts/run-dev-win.shsrc/openhuman/memory/tree/jobs/handlers/mod.rs
Addresses CodeRabbit's new actionable on the SC2155 fix from 09331d8: if `PNPM_EXE` somehow resolves to a bare filename, `dirname` returns ".", and prepending "." to PATH injects the current working directory — bad posture on a Windows dev machine where cwd often holds untrusted artifacts. Skip the prepend when PNPM_DIR is empty or ".", log that we did so, and fall through. The absolute-path call sites elsewhere in this script still work; only Tauri's beforeDevCommand needed bare-name pnpm resolution. Co-Authored-By: Claude <noreply@anthropic.com>
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Review by @graycyrus
Solid PR — all three fixes are well-motivated by production evidence and the implementation is clean.
Walkthrough
| Area | Files | What |
|---|---|---|
| Rust core | ingest.rs |
DEFERRED → IMMEDIATE tx — correct fix for SQLite busy-handler bypass on read→write lock upgrades |
| Rust core | store.rs, tree_source/store.rs, handlers/mod.rs, jobs/mod.rs |
Tombstone tables + NOT EXISTS clauses across all 3 worklist/probe sites — properly synchronized |
| Scripts | run-dev-win.sh |
PATH prepend guard + dedup loop — defensive and well-commented |
Notes
- IMMEDIATE tx is strictly safer than DEFERRED here — acquires write lock at
BEGINwherebusy_timeoutapplies. No deadlock risk since the tx is short-lived. - Tombstone design (separate tables vs NULL sidecars) is the right call — keeps
*_embeddingsschema clean and theNOT EXISTSpredicates trivial. - All 3 SQL sites (handler batch worklist,
has_uncoveredruntime probe, migration probe) are properly updated with matchingNOT EXISTS … reembed_skippedclauses. The in-code cross-references make future drift obvious. - Test
reembed_backfill_tombstones_orphan_and_terminatespins the critical regression path: tombstone write → worklist exclusion → chain termination → migration probe agreement. - CodeRabbit's
.guard concern was addressed in e48ee70 with a soft-warning approach — appropriate since absolute-path call sites still work in the degenerate case.
No critical or major findings — nice work. Pending: unit tests for the IMMEDIATE-tx path under simulated lock contention (acknowledged in PR description as follow-up before un-drafting).
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Looks good, nice work!
3 participants
Summary
memory_tree::ingest::persist()uses an IMMEDIATE-tx for the read-then-write transaction. The previous DEFERRED tx bypassed the 15 sbusy_timeouton lock upgrades (SQLite returnsSQLITE_BUSYimmediately for read→write upgrade conflicts, by design, to avoid deadlocks), so every Gmail per-message ingest failed under worker contention. Live evidence: 13/13 fails → 0/N on the verification run.reembed_backfillrunaway loop fixed: terminal-skip rows (body file missing on disk, embed wrong-dim, embed err) are now persistently tombstoned in two new*_reembed_skippedtables; the worklist/probe queries (handler batch +has_uncoveredruntime + migration) exclude them viaNOT EXISTS. Live verification on a dev workspace: 308 orphan chunks each attempted exactly once,chain completereached in ~2 min — replaces a runaway that previously generated ~128 kbody read failed; skippingwarns across ~8 k batch defers for ~16 stuck chunks because the "skipping" was log-only with no persistent exit gate.scripts/run-dev-win.sh— two Windows-contributor bugfixes forpnpm dev:app:win: (a) prependdirname($PNPM_EXE)to PATH so Tauri'scmd /S /C "pnpm run dev"can resolve barepnpm; (b) deduplicate PATH right beforetauri devso the MSYS→Windows env-conversion doesn't overflow the process-env block (without dedup, the spawned cmd inherits an EMPTY PATH).Problem
Observed in production-mode use on Windows over a long debugging session:
ingest_emailwrite fails withdatabase is locked. The connection hasbusy_timeout(15s)configured, but the failures arrive in milliseconds, not seconds. Cause: a DEFERRED transaction takes a read lock then tries to upgrade to write on the first INSERT; under contention with the memory-tree worker pool, SQLite returnsSQLITE_BUSYimmediately for the lock upgrade and does NOT invoke the busy handler (documented deadlock-avoidance behaviour). The 15 sbusy_timeoutis bypassed; ingest fails fast every time;composio_syncnever persists anything.reembed_backfillzombie loop — runaway re-selection of orphan chunks that can never embed (their body files are missing on disk, e.g. after a workspace path move). Generates ~128 k log warns / hour and burns one worker slot every 5 s indefinitely. Root cause: the*_embeddingstables only record successful embeddings; the worklist queryWHERE NOT EXISTS embeddingskeeps returning the same failing chunks because the "skipping" branch in the handler is log-only — no persistent state is written to exit the worklist.pnpm dev:app:windoesn't start cleanly on Windows —beforeDevCommanddies with'pnpm' is not recognized. Two underlying causes: (a)pnpm's install directory was never added to PATH (the script otherwise only calls pnpm by absolute path); (b) the bash-side PATH stacks the system PATH ~5× over (vcvars rebuild +/etc/profilere-runs + pnpm.binlayering), and the MSYS→Windows env-block conversion drops the entire PATH variable when it exceeds the limit — so the spawnedcmdinherits an empty PATH (not "pnpm missing" — every Windows PATH entry missing, including%SystemRoot%\System32, so evenwherereturns "not recognized").Solution
memory_tree/ingest.rs:216rusqlite::Transaction::new_unchecked(conn, TransactionBehavior::Immediate)instead ofconn.unchecked_transaction()(deferred). Locks at BEGIN where the busy handler applies; writers serialize and wait instead of failing fast.memory_tree/store.rs,tree_source/store.rs,jobs/mod.rs,jobs/handlers/mod.rsCREATE TABLE IF NOT EXISTSrows (mem_tree_chunk_reembed_skipped,mem_tree_summary_reembed_skipped), two helpers (mark_chunk_reembed_skipped,mark_summary_reembed_skipped), three worklist/probe SQL queries get anAND NOT EXISTS … reembed_skippedclause (handler batch inhandlers/mod.rs, thehas_uncoveredruntime probe injobs/mod.rs, thehas_uncoveredmigration probe instore.rs). Handler skip branches now also write a sentinel vialet _ = mark_*_reembed_skipped(...)(best-effort: if the sentinel write itself fails, the row simply gets retried on the next batch — desirable fallback).scripts/run-dev-win.shPNPM_EXEis resolved,export PATH="$(dirname "$PNPM_EXE"):$PATH". (b) Right before"$PNPM_EXE" tauri dev, an in-script first-seen-wins dedup loop collapses PATH (entries are clean/c/...POSIX so:-split is safe). Adds two[run-dev-win]info-level log lines so the next contributor can see at a glance what happened (pnpm dir prepended to PATH: ...,PATH de-duplicated: 317 → 71 entries).Design choices worth flagging for reviewers:
*_embeddingstable hasvector BLOB NOT NULLanddim INTEGER NOT NULL, so a NULL sentinel doesn't fit the schema, and an empty-blob/zero-dim sentinel would pollute the embedding readers (they'd need a special-case for "not a real embedding"). A separate small table keeps the embedding semantics clean and the worklist query'sNOT EXISTSpredicate trivial.LIMIT ?; the runtime + migration probes areSELECT EXISTS(... OR ...)); abstracting them would either lose query-shape clarity or require a builder. The duplication is small and the in-place comments cross-reference each other, so future drift would be obvious.Submission Checklist
chain completereached, 0 ingest lock failures observed in the new run) but unit-test additions for the new SQL paths are pending. The PR is opened as DRAFT for this reason. There IS an existing testreembed_backfill_repopulates_then_completesinhandlers/mod.rs:1062that already exercises the batch-defer-then-complete cycle — it should still pass with the newNOT EXISTSclause (the sentinel is only written on failure paths the test doesn't trigger), but I have not run it explicitly post-change.pnpm dev:app:winis unaffected for contributors whose machines were already correctly configured (the two fixes are additive — they only matter when pnpm's dir isn't otherwise on PATH and when PATH is otherwise overflowing).Impact
Runtime / platform: desktop, all platforms for the Rust changes; the
run-dev-win.shchange is Windows-only.Performance: net positive. Gmail ingest no longer drops messages and serializes correctly; the reembed worker no longer wastes one slot every 5 s on the same orphan reads.
Migration / compatibility: two new SQLite tables added via the existing
CREATE TABLE IF NOT EXISTSSCHEMAconst path. Nouser_versionbump; tables materialise lazily on nextwith_connection. Backward-compatible: older binaries opening the DB will simply ignore the new tables.Security: no privilege changes.
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/memory-tree-and-win-dev9b9f54eaValidation Run
pnpm --filter openhuman-app format:checknot run — Windows working tree has the documented ~600-file CRLF/LF drift;format:checkwould rewrite unrelated files. Pre-push hook bypassed via--no-verifyper the same documented pattern.pnpm typechecknot run — no TypeScript surface changed in this PR.reembed_backfill_repopulates_then_completesintegration-style test injobs/handlers/mod.rs:1062should still pass with the newNOT EXISTSclause (it triggers the success path the sentinel is not written on); I have not run it explicitly post-change.cargo checkran implicitly viapnpm dev:app:winrebuild — 0 errors.cargo fmtnot explicitly run; will run in the follow-up that adds tests.app/src-tauri/Rust changes in this PR.Validation Blocked
command:cargo fmtand unit-test additions for the IMMEDIATE-tx and reembed-skip pathserror:n/a — not blocked technically; bundled with the test-coverage follow-up before un-drafting.impact:PR is opened as DRAFT; reviewer feedback welcome on the schema + worklist-SQL approach before I invest in the test suite.Behavior Changes
persist().reembed_backfillchain terminates instead of looping forever on orphan chunks — terminal failures are persistently tombstoned and excluded from the worklist.pnpm dev:app:winactually starts on a fresh Windows checkout without manual workarounds.[memory_tree::jobs] reembed_backfill: chunk … body read failed; skipping(was ~128 k warns / run before, now once per orphan per signature).pnpm dev:app:winand have a Tauri shell open.Parity Contract
mem_tree_chunk_reembed_skipped/mem_tree_summary_reembed_skippedare created idempotently — older code opening the DB pre-this-change works unchanged; rolling back this commit removes only the additive tables.NOT EXISTS … reembed_skippedclauses are pure subtraction from the worklist — every chunk that previously appeared still appears unless it has a terminal-failure sentinel, which only the new handler code can write.mark_*_reembed_skippedis best-effort (let _ =): a transient write failure does not crash the batch; the row simply gets retried on the next batch (desired fallback).busy_timeoutalready governs the wait. Existing successful writers under no contention behave identically.run-dev-win.shchanges are additive and only fire conditions that already produced errors (pnpm-not-on-PATH; PATH overflow). Contributors whose machines didn't hit those conditions see no behaviour change.has_uncoveredruntime probe +has_uncoveredmigration probe) to avoid drift between sites; each unconditionally addsAND NOT EXISTS … reembed_skippedso the three remain semantically aligned.Duplicate / Superseded PR Handling
🤖 Generated with Claude Code
Summary by CodeRabbit