fix(reindex): the remaining review findings — wrong guards, missing bounds, and a setting that lied

[kevintseng]

fix(reindex): the remaining review findings — guards that were wrong, bounds that were missing, and a setting that lied

The eight-lane review of #185 left sixteen findings below P0. This closes them,
at the root rather than at the symptom.

Guards that answered the wrong question:

• getVectorGenerationInfo collapsed "no rebuild in progress" and "the marker
  exists but I could not read it" into one null, and the caller treats null as
  licence to DROP the staging table. So a JSON.parse failure, a field of the
  wrong type, or any SQLite read error silently threw away every embedding a
  previous run had produced. It is now a tri-state, and an unreadable marker over
  staged work REFUSES: resuming could merge two embedding spaces, discarding
  destroys paid work, and neither is safe to choose silently. memesh reindex --discard-generation is the deliberate way out, and doctor now reports the
  half-built index rather than leaving it invisible on disk.
• A resume skipped an entity whenever a row was already staged, on presence
  alone, so an entity edited between an interrupted run and its resume kept the
  vector built from its old text — undetectable afterwards, because the row was
  there. Each staged row now records a hash of what it was embedded from.
  already_staged became its own counter in the process: a resumed run used to
  report "900/900 entities embedded" after one request, contradicting what
  embedded documents itself to mean.
• SQLite's TRIM strips U+0020 only; JS .trim() also strips tab and newline. The
  loop and the database therefore disagreed about the same entity — permanently
  "nothing to embed" to one, permanently owed a vector to the other — so every
  full reindex reported a memory missing forever.

Bounds that were declared but not enforced:

• No circuit breaker. A provider that stopped answering was still asked about
  every remaining entity, up to ~91.5s each, printing one identical failure per
  entity — including for a 401, where the code's own comment says retrying
  "spends the rate budget on a certainty". Five consecutive failures now end the
  run; the generation survives, so the next run resumes. Backoff is exponential
  rather than linear, so a rate limit is backed away from instead of re-arrived
  at, and its ceiling is its own constant instead of a second use of
  PROVIDER_TIMEOUT_MS.
• res.json() was awaited in the caller, outside the retry. A provider that
  returned headers and then stalled the body was aborted correctly at 30s, but
  the AbortError came from the parse where there is no retry and no attempt
  counter, arriving as an indistinguishable null. That is exactly the failure the
  timeout was added to catch, so the parse moved inside.
• The swap now re-counts the staging table INSIDE its transaction and refuses to
  publish a short index. Labelled in the source as unpinned: the condition has no
  reachable trigger to construct, so it is defence in depth for the one
  irreversible step, kept and marked rather than given a test that proves nothing.

Reporting that described the wrong thing:

• memesh reindex built its verdict from a count taken against whatever index is
  LIVE, so a refused swap measured the old, complete-by-construction index: exit
  0 and a tick, while its own output said the new index was not switched in.
  ReindexResult gained generationSwapped and abortedAfter, both in --json.

A setting that lied, removed rather than implemented:

• embedder.model was settable and documented in three READMEs and never
  reached the embedding call. Wiring it through would have introduced the fault
  this release closes — the width comes from the provider, so another width could
  never be rebuilt against and the same width would mix two embedding spaces with
  no signal — so the key is gone and the READMEs now state that each provider
  pins its own model and width. Those READMEs also still claimed the index
  "rebuilds automatically on the next write", which feat: reindex builds the new vector index beside the old one (generation swap) #185 made false.

Verification, run in this session against a throwaway HOME:

node scripts/run-tests-isolated.mjs exit=0
Test Files 154 passed (154) / Tests 2243 passed (2243); no "Errors" line
npm run typecheck exit=0
npm run build exit=0
node scripts/audit/verification-audit.mjs exit=0
C1 new=0 C3 new=0 C4 new=0 C5 new=0 C6 new=0 C7 new=0

Break-test, each mutation applied then written back and re-verified:
tri-state collapsed back to null -> KILLED
resume ignores the content fingerprint -> KILLED
circuit breaker removed -> KILLED
SQLite TRIM back to spaces only -> KILLED
body parse moved back outside the retry -> KILLED
Retry-After cap removed -> KILLED
redirect: error removed -> KILLED
swap re-count removed -> SURVIVED (no test; labelled in source)

Audit baseline: 16 C5 entries re-keyed. Every one was matched by comparing the
exact statement text at the new line against the same line in HEAD — a re-key,
not a re-triage. No entry was reclassified and none was pruned.

New coverage: tests/core/embedder-provider-fetch.test.ts, which the provider path
had none of. Two shapes are recorded in its header because both make a test pass
for the wrong reason: an ok: false stub must carry real headers (the retry path
reads retry-after), and the 30s timeout equals vitest's own testTimeout, so that
branch can only be reached by rejecting with an Error named TimeoutError.