Skip to content

fix: correctness & data-integrity fixes across user profiles and turso sharding - #252

Open
phil-lipp wants to merge 5 commits into
tickernelz:mainfrom
phil-lipp:fix/correctness-data-integrity
Open

fix: correctness & data-integrity fixes across user profiles and turso sharding#252
phil-lipp wants to merge 5 commits into
tickernelz:mainfrom
phil-lipp:fix/correctness-data-integrity

Conversation

@phil-lipp

Copy link
Copy Markdown

Summary

Ten correctness / data-integrity bugs found during a code-quality pass, grouped into five reviewable commits (each builds and typechecks independently). No behavior changes beyond fixing the defects; no public API changes.

bun run typecheck clean, bun test green (adds one regression test).

Fixes

user-profile-manager.ts

  1. Cross-user profile contamination via the cold-start buffer.
    coldBuffer was a single instance field on the manager singleton. Items buffered during embedding warm-up for one user drained into whichever user's mergeItems ran next, merging User A's preferences into User B's profile. Now keyed per profileId (Map); each merge drains only its own bucket. Legacy unattributed buffer files are dropped on load. (covered by the new test)
  2. learning_paths stripped on every write. createProfile / updateProfile rebuilt cleanedData as only {preferences, patterns, workflows}, silently discarding learning_paths set by buildLearningPaths — the Learning Paths injection feature was effectively dead. Carried through.

user-memory-learning.ts

  1. Unawaited learning-captured marker → duplicate analysis. The retry-exhausted branch didn't await markMultipleAsUserLearningCaptured, so finally cleared isLearningRunning mid-write; the next cycle re-fetched and re-analyzed the same prompts (token burn), and the rejection was unhandled.
  2. Fire-and-forget evolveAndUpdate races serialization (lost update). It mutates item.description/centroid in place but was called .catch(()=>{}), racing the JSON.stringify in updateProfile — the evolved description was included or lost nondeterministically. applyValidations is now async and awaits the evolve so mutation completes pre-serialization.
  3. No catch around stored-JSON parses. performUserProfileLearning had try/finally but no catch; JSON.parse of a corrupt profileData row rejected the promise (unhandled at the fire-and-forget call site). Added.

user-profile/ai-cleanup.ts

  1. Hallucinated AI ids abort the whole cleanup. In the merged-group rebuild, a keeper id present only in the model's cleaned output (absent from originalById) threw on originalItem.frequency, aborting the entire run. Skip such groups. Also added normalizeAIMapping so a malformed mapping (kept/merged/removed) degrades to a no-op cleanup instead of throwing on .map/.filter/.includes.

user-profile/profile-context.ts

  1. Unguarded JSON.parse in the injection hot path. One corrupt profile row broke context injection for every request. Wrapped in try/catch; returns null on parse failure (mirrors loadColdBuffer).

turso/shard-manager.ts

  1. Orphaned shard registry row bricks writes. createShard committed the registry INSERT before initShardDb ran; if init threw (disk full, permissions) the row persisted pointing at an uninitialized file, so the next
    getWriteShard failed isShardValid and threw "incompatible or corrupt", blocking all writes to that scope. Now initializes the shard DB first (initShardDb is idempotent), then inserts.
  2. getShardByPath LIKE treats _ as a wildcard. WHERE db_path LIKE '%' || ? with a filename full of _ (user_<hash>_shard_N.db) matched any char and could return the wrong shard row. Anchored on the / separator with escaped LIKE metacharacters (ESCAPE '\').

turso/operation-lock.ts

  1. Unguarded unlinkSync in lock cleanup. readLiveLock called unlinkSync outside its try/catch; a race (already removed) or a Windows open handle threw ENOENT/EPERM out of assertNoTursoMigrationInProgress, falsely blocking writes. Wrapped it.

Tests

Adds tests/user-profile-cold-buffer-isolation.test.ts, asserting that items buffered for one profile during embedding cold-start never drain into another profile's merge, and that each bucket drains only for its own profile.

Notes for reviewers

  • The commits are split by subsystem and can be cherry-picked independently if you'd prefer to take a subset.
  • normalizeAIMapping intentionally coerces to safe empty arrays rather than throwing, so a bad model response results in a no-op cleanup (originals preserved) rather than an aborted run — the more data-preserving choice.

…ing_paths

The cold-start buffer was a single instance field on the profile-manager
singleton, so items observed for one user during embedding warm-up drained
into whichever user's mergeItems ran next — merging User A's preferences into
User B's profile. Key the buffer by profileId (Map) and drain only the current
profile's bucket; legacy unattributed buffer files are dropped on load.

Separately, createProfile/updateProfile rebuilt cleanedData as only
{preferences, patterns, workflows}, silently stripping learning_paths on every
write and rendering the Learning Paths injection feature dead. Carry the field
through.
rebuildProfileUsing dereferenced originalItem unconditionally in the merged-group
branch, so a keeper id the model hallucinated (present only in its mapping, not
in originalById) threw and aborted the entire cleanup run. Skip such groups, and
normalize the AI-controlled mapping (kept/merged/removed) to well-formed arrays so
a malformed response degrades to a no-op cleanup instead of throwing.

getUserProfileContext parsed the stored profileData with no guard, so one corrupt
row broke context injection for every request. Wrap in try/catch and return null.
…ialization

Three fire-and-forget/unawaited hazards in the learning cycle:

- The retry-exhausted branch did not await markMultipleAsUserLearningCaptured, so
  finally cleared isLearningRunning while the write was in flight; the next cycle
  re-fetched and re-analyzed the same prompts (token burn), and the rejection was
  unhandled. Await it.
- evolveAndUpdate mutates item.description/centroid in place but was called
  fire-and-forget, racing the JSON.stringify in updateProfile — the evolved
  description was included or lost nondeterministically. Make applyValidations
  async and await the evolve so mutation completes pre-serialization.
- performUserProfileLearning had try/finally but no catch; JSON.parse of a corrupt
  profileData row rejected the promise (unhandled at the fire-and-forget site). Add
  a catch that logs and returns.
…ck cleanup

- createShard committed the registry INSERT before initShardDb ran; if init threw
  (disk full, permissions) the row persisted pointing at an uninitialized file, so
  the next getWriteShard failed isShardValid and threw 'incompatible or corrupt',
  blocking all writes to that scope. Initialize the shard DB first (it is
  idempotent), then insert.
- getShardByPath matched db_path with LIKE '%' || filename, so the underscores in
  shard names (user_<hash>_shard_N.db) acted as single-char wildcards and could
  match the wrong row. Anchor on the '/' separator and escape LIKE metacharacters.
- readLiveLock called unlinkSync outside its try/catch; a race (already removed) or
  a Windows open handle threw ENOENT/EPERM out of assertNoTursoMigrationInProgress
  and falsely blocked writes. Wrap it.
Asserts that items buffered for one profile during embedding cold-start never
drain into another profile's merge, and that each bucket drains only for its own
profile.
@phil-lipp
phil-lipp force-pushed the fix/correctness-data-integrity branch from 999d51c to e79af24 Compare August 13, 2026 16:39
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