feat: write-behind capture (#50) - #56
Merged
Merged
Conversation
mibrahimdev
added a commit
that referenced
this pull request
Aug 22, 2026
…, session commit (#50) Addresses both reviewers' REQUEST-CHANGES on #56. - Channel now uses BufferOverflow.DROP_OLDEST so the crash-tail (newest events) survives backpressure, never the oldest. Pinned by a direct-channel test. - start() guarded by an AtomicBoolean (idempotent — no double flusher). - Persistence bootstrap made thread-safe with an AtomicBoolean CAS (`synchronized` is JVM-only, unavailable in commonMain). - stop() drains the pending channel + in-flight batch (channel.close() then join), then cancels the scope and closes the SqlDriver. - sessionId memoized only AFTER the batch transaction commits; reset on rollback so a later batch re-derives a rolled-back session row. - Per-batch transaction wrapped in try/catch so one failure never kills the flusher coroutine. - Tests: DROP_OLDEST overflow semantics, stop() drains pending batch; two-session test no longer stop()s (stop now closes the shared driver). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 tasks
Slice 2 of the flight-recorder epic. Events now flow off the in-memory
ring buffer into the on-device DB, so logs survive process death.
- SharinganStore gains an internal `onRecord` seam, invoked after the
unchanged lock-free CAS append (only while recording). No public change.
- PersistenceController owns CoroutineScope(SupervisorJob()+Dispatchers.Default),
wires `onRecord = { channel.trySend(it) }` on a bounded channel, and drains
it with a single flusher coroutine that writes each batch in one SQLDelight
transaction (size ~50 / 250 ms, whichever first).
- Lazy session row created on the first flushed event of the launch.
- internal @serializable EventDto (Http/Mqtt/Ble) with fromEvent() encode-only;
public event ABI untouched. Stored as the event payload_json blob.
- Persisted event PK is globally unique across sessions: session id is
timestamp+random, and the event row id prefixes the raw EventIds value with
the session id (EventIds resets each launch, so the raw value is not safe
as a cross-session PK).
- Hands-free on Android via the existing manifest ContentProvider.
- Tests: burst > ring capacity all persisted, batching (not one-write-per-event),
record() unchanged when persistence off, cross-session id uniqueness.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, session commit (#50) Addresses both reviewers' REQUEST-CHANGES on #56. - Channel now uses BufferOverflow.DROP_OLDEST so the crash-tail (newest events) survives backpressure, never the oldest. Pinned by a direct-channel test. - start() guarded by an AtomicBoolean (idempotent — no double flusher). - Persistence bootstrap made thread-safe with an AtomicBoolean CAS (`synchronized` is JVM-only, unavailable in commonMain). - stop() drains the pending channel + in-flight batch (channel.close() then join), then cancels the scope and closes the SqlDriver. - sessionId memoized only AFTER the batch transaction commits; reset on rollback so a later batch re-derives a rolled-back session row. - Per-batch transaction wrapped in try/catch so one failure never kills the flusher coroutine. - Tests: DROP_OLDEST overflow semantics, stop() drains pending batch; two-session test no longer stop()s (stop now closes the shared driver). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
timestamp+Random collided on Kotlin/Native (same millis + Native Random.Default returning the same value on rapid successive calls), so the two sessions in the test produced one PK and the second insert hit a UNIQUE violation (swallowed by the flusher's try/catch). kotlin.uuid.Uuid.random() is collision-free on every target; started_at stays currentTimeMillis(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The constant "sharingan-test.db" with inMemory=true mapped to a single file:...?cache=shared in-memory DB on Kotlin/Native, so every createTestDriver() in the test binary shared one database and earlier tests leaked ~700 rows into the two-sessions test. A per-driver UUID name gives each test a private DB. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Enable PRAGMA foreign_keys / setForeignKeyConstraintsEnabled on Android, iOS driver configuration, and JVM test drivers. Add deleteSession query and a cascade test that fails when FKs are off. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Stamp a deadline when the first event enters an empty batch and use the remaining time in withTimeoutOrNull. A slow steady stream now flushes within the configured interval instead of waiting for the batch size. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Give PersistenceController a three-method lifecycle: start() wires the seam, stop() drains and leaves the driver open for readers, close() stop()s then cancels the scope and closes the driver. Tests inject the driver through an internal constructor; production uses the no-driver constructor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…50) Move SQLDelight schema, drivers, persistence controller and tests into a new :sharingan-db module. The controller becomes a generic PersistenceController<T> that receives a (T) -> EventRow mapping, keeping JSON encoding off the hot record() path. :sharingan consumes :sharingan-db as an implementation dependency, so the SQLDelight-generated types are no longer exported to the iOS framework header. - EventDto and toRow stay in :sharingan, package dev.sharingan.internal - Add SharinganDbContext for the Android context seam - Add Now.* equivalents for :sharingan-db's own time source - Update CI, BCV config, and release docs for the third artifact Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#50) Add KDoc warnings to stop() and close() so slice 4's configure()/shutdown path knows that events submitted after stop() are silently dropped unless the caller detaches store.onRecord first. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pin the JSON encoding of HttpEvent / MqttEvent / BleEvent via EventDto in :sharingan, including the redacted-header value. Uses the same json instance that toRow() uses (now internal for test visibility). No decode round-trip — toEvent() is slice 3. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#50) The caller-supplied (T) -> EventRow mapping can throw (e.g. JSON encode of an unbounded body). Move it inside the existing batch try/catch so the exception drops one batch instead of escaping runFlusher and crashing the host process. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
withTimeoutOrNull can prompt-cancel after taking an element from the
channel, losing events under backpressure. Replace the timeout path with
select { channel.onReceiveCatching { ... }; onTimeout { ... } } so the
receive and timeout are atomic.
The race itself is not deterministic in a unit test; the new focused test
pins the deadline-flush path that the select exercises.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stop (#50) A concurrent stop() could read flusherJob before start() assigned it, skip the join, and tear down the scope while a transaction was in flight. Replace the AtomicBoolean + var pair with a single AtomicReference<Job?> used as the start/stop gate; stop atomically reads-and-clears it before joining. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t DROP_OLDEST (#50) Expose channelCapacity through the internal constructor so tests can exercise backpressure without relying on production defaults. Rename the 500-event burst test to reflect that it stays within the channel, and add a focused DROP_OLDEST test that submits before start() so the flusher cannot drain during the burst. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add focused contract tests showing that the in-memory store forwards every accepted event to the internal persistence seam, skips forwarding while paused, and still forwards events that are later evicted by the ring buffer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewers found that the finding-#3 AtomicReference rewrite reintroduced finding-#2 one method up: start() launched the flusher eagerly before the CAS, so a redundant start() cancelled a coroutine that could already be suspended in channel.receive(), losing events. - Fix A: launch the flusher with CoroutineStart.LAZY and call job.start() only inside the CAS-won branch. Drop the losing job's cancel() entirely. - Fix B: add a terminal `stopped` state. stop() sets it; start() after stop() throws IllegalStateException. Update KDoc on start()/stop() to state single-use semantics and remove the now-wrong 'Idempotent' label. - Fix C: document that the `select`/`onTimeout` usage is ExperimentalCoroutinesApi and load-bearing for the published module. - Optional nit: collapse readAndClearFlusherJob() to flusherJob.exchange(null). Add a focused TDD test proving a redundant start() dispatches exactly one flusher and loses no events, and rewrite the vacuous start/stop-cycle test into an assertion that start()-after-stop() throws. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eterministic test (#50) - Fix 1 (iOS CI blocker): replace the JVM-only CountingDispatcher/Runnable approach in PersistenceControllerTest with an internal `flusherStartCount()` seam backed by an AtomicInt. This removes the `java.lang.Runnable` reference that broke Kotlin/Native compilation. - Fix 2 (single-use hole in stop()): always close the channel before reading and joining the flusher job. A concurrent start() that wins the CAS after stop() sets the terminal flag now starts on a closed channel and exits cleanly instead of leaving a live flusher behind. - Fix 3 (deterministic double-start test): assert `flusherStartCount() == 1` after two start() calls instead of string-matching a kotlinx.coroutines internal class name. - Optional nit: use `error(...)` for the single-use guard (still throws IllegalStateException). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The assertion that only one flusher started was read before the flusher had demonstrably entered its body. Move it after the flushed.await() so the count is deterministic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cut comments that restated the code or narrated review history; kept the non-obvious why-notes (LAZY start, atomic select, map-inside-try, DROP_OLDEST, UUID-not-Random, the stop() seam WARNING). Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment-only edits in PersistenceController, EventDto, Persistence, and PersistenceControllerTest per Vigil + Sage cut-list. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
:sharingan-db ships to Maven Central, so its public surface must be guarded by apiCheck like any other published module. Remove the ignoredProjects exemption, commit the generated api/*.api dumps, and verify apiCheck passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three statements predate the flight recorder: ARCHITECTURE's overview claimed nothing is persisted, section 5.4 described persistence as hypothetical/opt-in, and CONTEXT.md's Store entry claimed memory-only. All three now describe what ships: an in-memory ring buffer (300) that is also mirrored to a SQLite flight recorder via the write-behind seam (SharinganStore.onRecord -> PersistenceController), on by default in debug, with request/response bodies never written to disk. CONTEXT.md gains Flight Recorder, Write-Behind Seam, and Run entries, and the Capture entry notes redaction applies before disk too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#50) Five more places still claimed "memory-only / never persisted": the README feature bullet and SharinganStore note, ARCHITECTURE's design properties and known-limitations entries, and AGENTS.md lines 3 and 106. All now say what ships: the ring buffer stays memory-only and is still cleared on process death; events are additionally mirrored to the on-disk SQLite flight recorder (debug only), which survives, and bodies are never written to disk. AGENTS.md and llms.txt are a hand-maintained mirror — regenerated llms.txt from AGENTS.md and confirmed byte-identical before committing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Capture entry gained a stray blank line before its _Avoid_ line; every other entry has them adjacent. Backtick `session` in the Run entry so it reads as the schema table name rather than the term reserved for the v2 epic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UBhi6Qx8rdo1aX9EnmtEJS
…disk (#50) B1: SharinganStore.clear() now invokes an onClear seam; Persistence wires it to a new PersistenceController.clear(). The clear travels as a command on the same channel the writes use, so the single flusher coroutine deletes rows at an exact point in the write order — a pending in-flight batch cannot resurrect events after a clear. B2: toRow() strips HttpDto request/response bodies and Mqtt/Ble payloads on the persistence path only (design default persistBodies = false); EventDto stays capable of carrying bodies for the slice-5 opt-in. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The seam had no lifecycle. Persistence.start() wired store.onRecord and forgot both the controller and the store, so nothing could unwire it and 'started' latched true for the process lifetime. - Wire the seams BEFORE starting the flusher. start() is LAZY so the old order was harmless, but the dependency is now encoded rather than implied. - Add Persistence.stop(): unwires onRecord/onClear, closes the controller, and resets 'started' so a later start() works. Retains the store it wired. - @volatile on both seam properties (kotlin.concurrent.Volatile, so it applies on Native as well as the JVM). They are written at process start and read on the capture path. Left deliberately: the dropped-batch report stays a println, since the project has no logging facility and a debug-only recorder does not justify inventing one. Both simplifications carry ponytail: comments naming the ceiling. Not unit-tested, and the docblock says why: start() builds a real driver, and DriverFactory.create() on Android needs the ContentProvider-installed Context that a JVM unit test cannot supply. The alternative was Robolectric or a controller-injection seam, neither justified by straight-line wiring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UBhi6Qx8rdo1aX9EnmtEJS
The BCV gate added in 87f9e71 caught the ABI widening from the clear() work: PersistenceController.clear() (called cross-module by :sharingan) and the SQLDelight-generated deleteAllEvents(). Both are intended. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UBhi6Qx8rdo1aX9EnmtEJS
develop gained a ktlint ruleset (#58) after this branch forked: standard formatting rules plus a custom no-multi-line-comment rule. Apply ktlintFormat and condense every multi-line // run this branch introduced into a single line, per the AGENTS.md comments policy. KDoc is exempt, so the Persistence lifecycle docblock is unchanged. Also applies the ktlint plugin to :sharingan-db. The module was created on this branch before the ruleset existed, so it was escaping the lint gate the same way it was escaping the BCV gate — adding it surfaced eight violations that were previously invisible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UBhi6Qx8rdo1aX9EnmtEJS
mibrahimdev
force-pushed
the
feat/50-write-behind
branch
from
August 31, 2026 18:46
4ff585b to
377e2bb
Compare
This was referenced Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slice 2 of the flight-recorder epic (#27), on the #49 DB foundation: captured events are written behind the hot path to SQLDelight, so they survive process death.
:sharingan-dbmodule — persistence lives off the public ABI;:sharinganand:sharingan-noopAPI dumps are unchanged.internal var onRecordseam onSharinganStore, fired after the unchanged lock-free CAS append, only while recording.PersistenceControllerowns itsSqlDriverandCoroutineScope. The seamtrySends onto a boundedDROP_OLDESTchannel (overflow keeps the crash-tail); one flusher drains it, batching by size-or-time into one transaction per batch. Foreign keys on, so session deletes cascade. Single-use —start()afterstop()throws.EventDto— internal@Serializablemirror, encode-only. Public event ABI untouched."<sessionId>-<rawId>", session idsession-<uuid>; session row created lazily on first flush.Bootstrap is Android-only this slice (manifest-merged ContentProvider); the controller is platform-agnostic and iOS gets
Sharingan.configure()in slice 4.