Skip to content

Serialise apply_delta to fix concurrent-ingest XP loss (~8%) - #38

Merged
ppXD merged 1 commit into
mainfrom
fix/apply-delta-race
May 19, 2026
Merged

Serialise apply_delta to fix concurrent-ingest XP loss (~8%)#38
ppXD merged 1 commit into
mainfrom
fix/apply-delta-race

Conversation

@ppXD

@ppXD ppXD commented May 19, 2026

Copy link
Copy Markdown
Owner

Summary

Live audit on a heavy 獨角獸 (Unicorn) session showed:

  • sum(xp_event) = 988
  • pet_state.total_xp = 910
  • 78 XP / ~8% loss

Root cause: StateManager::apply_delta does a non-atomic read-modify-write on pet_state. Concurrent ingest pairs race β€” both tasks read the same prior xp, both write prior + own_delta, the later write overwrites the earlier. The xp_event row is still written (unique by UUID, no dedup conflict), so the events database disagrees with the state row.

Fix

Single tokio::sync::Mutex<()> field on StateManager, locked across the whole read-modify-write in both apply_delta and rebuild. petpet runs at most one active pet at a time, so a global mutex is the right granularity β€” contention is bounded by live ingest rate (a few events/sec worst-case).

If we ever support multi-pet concurrent ingest, swap for a per-pet mutex map (HashMap<pet_id, Arc<Mutex<()>>> behind a parent RwLock).

Why not BEGIN IMMEDIATE transaction?

Cleaner long-term, but DbHandle exposes single-statement execute shapes, not multi-statement transaction blocks. The Mutex is mechanically equivalent here β€” one DbHandle, one active pet, write contention = mutex contention. Worth revisiting if we ever:

  • Support multi-pet concurrent ingest
  • Add an external SQL client that bypasses the engine's serialisation

Regression test

apply_delta_serialises_concurrent_ingest:

  1. Spawn 200 concurrent apply_delta(+1) tasks against one pet
  2. Await all, read final pet_state.total_xp
  3. Assert == 200

Pre-fix: probabilistic failure (some tasks would race and lose XP). Post-fix: 5/5 reruns clean, 309/309 lib tests pass.

πŸ€– Generated with Claude Code

@ppXD
Serialise apply_delta to fix concurrent-ingest XP loss
3336d46 
# The race

`StateManager::apply_delta` does a read-modify-write on `pet_state`:

    let prior = self.db.get_pet_state(pet_id).await?;       // READ
    let xp_after = prior.total_xp + delta;
    self.db.upsert_pet_state(pet_id, xp_after, ...).await?; // WRITE

With concurrent ingest the read+write isn't atomic across tasks:

    Task A: read pet_state.total_xp = 100
    Task B: read pet_state.total_xp = 100   ← same snapshot
    Task A: write 100 + 10 = 110
    Task B: write 100 + 5 = 105             ← overwrites A's 110

A's 10 XP is "lost" β€” the `xp_event` row is still written (different
UUID, different dedup key), so `sum(xp_event)` > `pet_state.total_xp`.
The pet's displayed level / stage is computed from the (stale) state,
so the user sees ~8% fewer XP than the events database implies.

Empirically reproduced: a 1.5-hour heavy Claude-Code Unicorn session
ended with `sum(xp_event)=988` but `pet_state.total_xp=910` β€”
78 XP / 7.9% loss.

# The fix

Single tokio Mutex held across the whole read-modify-write inside
`apply_delta` (and `rebuild`, which has the same shape). petpet
runs at most one active pet at a time, so a global mutex is the
right granularity β€” contention is bounded by the live ingest rate
(handful of events per second worst-case).

If multi-pet concurrent ingest ever lands, swap for a per-pet
mutex map (HashMap<pet_id, Arc<Mutex<()>>> behind a parent RwLock).

# Regression coverage

New test `apply_delta_serialises_concurrent_ingest`:

  1. Spawn 200 concurrent `apply_delta(+1)` tasks against one pet
  2. Await all, then read final `pet_state.total_xp`
  3. Assert == 200 (pre-fix: would be < 200 with high probability)

309/309 lib tests pass; race regression stable across 5 reruns.

# Why not SQL `BEGIN IMMEDIATE` transaction?

Cleaner long-term, but would require wider plumbing changes (DbHandle
exposes `execute` shapes, not multi-statement transaction blocks).
The Mutex approach is mechanically equivalent in this codebase since
there's only one DbHandle and one active pet β€” write contention
is logically equivalent to mutex contention. Worth revisiting if we
add multi-pet concurrent ingest OR an external SQL client that
might bypass the engine's serialisation.
@github-actions github-actions Bot added the fix Auto-applied by .github/workflows/auto-label.yml label May 19, 2026
@ppXD
ppXD merged commit afc34a4 into main May 19, 2026
4 checks passed
@ppXD
ppXD deleted the fix/apply-delta-race branch May 19, 2026 05:06
ppXD added a commit that referenced this pull request May 19, 2026
@ppXD
Add KingKong builtin template (Hard mode, ~2.5Γ— Sun difficulty) (#39)
57b0092 
New builtin pet template targeting the "much harder" slot above Sun.
The image set (10 sprites from black-stone egg through cyborg
legend) lives at templates/builtin/kingkong/stages/stage_*/sprite.png.

# Difficulty design β€” ~2.5Γ— Sun, achieved through rules + curve

The rule_mult clamp floor of 0.5 means usage rules can't go any
lower than Sun's. So extra difficulty comes from two compounding
levers:

  1. Activity rules β€” most cut to zero, milestones halved:
       session_start    2 β†’ 1
       user_prompt      1 β†’ 0     ← chat doesn't reward
       assistant_stop   0 (unchanged from Sun)
       tool_success     0 (unchanged)
       task_completed   7 β†’ 3     ← halved milestone
       subagent_stop    5 β†’ 2     ← halved milestone
       pre_compact      1 β†’ 0     ← eliminated
     Idle-chat dominance is gone; only deliberate progress moves
     the meter (tasks, subagents) plus AI token usage.

  2. Level curve β€” 1.25Γ— Sun via formula
        cum(L) = 200 + 188Β·(L-1) + 3Β·(L-1)Β²
     L1 = 200 (hatch anchor, consistent across all templates)
     L99 = 47,436 (Sun: 37,950, ratio 1.250Γ—)

Per-conv XP projection vs Sun (using PR #37 + #38 baselines):

  Profile       Sun XP/conv  KingKong XP/conv  Combined harder
  Light (5t)    12           ~6                ~2.5Γ— total time
  Medium (10t)  29           ~14               ~2.6Γ—
  Heavy (15t)   44           ~21               ~2.5Γ—

Stage triggers identical to Unicorn/Sun (L0/1/7/15/25/40/55/72/86/99).

# Display ordering

Adds `display_order: Option<i32>` to TemplateMeta. Builtin ladder:

  unicorn   display_order=1   (Easy)
  sun       display_order=2   (Medium)
  kingkong  display_order=3   (Hard)

EggPicker.tsx sorts by source β†’ display_order β†’ name. Templates
without `display_order` (user-imported community ones) fall to the
tail and sort by name there.

# Visual + flavor

Labels: "Hard" (black/gold) + "Challenge" (gold/black) β€” gives the
row an immediately ominous chip pair vs Sun's warm Medium/Wukong.

Stage flavor reads as an evolution arc: obsidian egg β†’ young
primate β†’ silverback β†’ armoured stride β†’ first circuits β†’ cyborg
legend. Default pet name "KingKong"; species flavor "Forged in
obsidian, ascending in titanium."

# Coverage

  - `builtin_dirs_exist` now asserts kingkong/ is bundled in
    include_dir!
  - new `pick_template_kingkong_loads_and_creates_pet` integration
    test pins end-to-end load + snapshot (matches the unicorn /
    sun canaries β€” every builtin gets its own load smoke test)
  - 310/310 lib tests pass; desktop bundle + tsc clean
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

fix Auto-applied by .github/workflows/auto-label.yml

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ppXD