mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
b6afc6809d
* perf(storage): row-native sequence primitive + thread/turn append paths Reduces durable write pressure on the per-turn storage path that dominates p95 once the resource governor and journal mode are no longer the limit. - Add a `reserve_sequence` RootFilesystem primitive for path-local monotonic sequence allocation, implemented for libSQL, Postgres, and the in-memory backend (migration V32 adds the libSQL/Postgres sequence table), surfaced through the scoped dispatch fabric and capability set. - Rework thread storage to use sequence reservation + a finalized assistant-append path, collapsing the per-turn full-document rewrites in accept_inbound / append_assistant / finalize into smaller appends. - Carry the same append-native shape through the turn-state store and runner lease records, and update the stress harness's user-turn path to exercise the finalized-append flow. This lifts the storage portion of #5453, dropping that PR's resource-governor commits which are superseded by the already-merged #5447 (unlimited-budget durable-write skip). It builds on the WAL change in #5451. Measured (this machine, chat-turn, 30 ops/task, 200 users), WAL-only vs WAL + this change: c=8 : thread_store_writes p95 126.2ms -> 106.4ms ; throughput 33.6 -> 36.2 ops/s c=32: thread_store_writes p95 186.0ms -> 97.0ms ; throughput 19.5 -> 34.2 ops/s aggregate p95 276.4ms -> 183.6ms Throughput previously collapsed past c8 (19.5 ops/s at c32); with the append-native paths it holds flat from c8 to c32. Co-Authored-By: firat.sertgoz <firatsertgoz@alumni.sabanciuniv.edu> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb * docs(stress): WAL + storage-rework libSQL write-concurrency results Dated boundary update capturing the before/after for the WAL change (#5451) and the row-native sequence + thread/turn append-path rework (#5455), plus the headline c100 sweep: 100 concurrent writes complete with zero failures and p95 256.8ms / p99 294.3ms on this 4-core container. Includes the raw JSONL artifacts. Clearly labeled as a different machine from the M4 usable-boundary results so the numbers are not cross-compared. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb * fix(migrations): pin V32 checksum in checksums.lock Adds the V32__root_filesystem_sequences entry to migrations/checksums.lock so the released-migration immutability check passes. Checksum computed via refinery::Migration::unapplied(...).checksum() (verified by reproducing the existing V31 entry with the same method). Addresses the CodeRabbit review finding on #5455. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb * fix(threads,filesystem,turns): address PR #5455 review findings Resolves the real findings from the Gemini and CodeRabbit reviews: - Redaction/update of append-only finalized messages now works. Such messages live only in the per-thread append log (no individual file), so apply_message_update CAS-wrote to a nonexistent path and failed. Fix: materialize the message file on first mutation (CasExpectation::Absent), and make merge_message_append_events file-authoritative (a per-message file shadows its append-log entry) so the redacted record wins on reads. This matches read_message_versioned, which was already file-first. (Gemini critical / CodeRabbit major.) - Sequence counters are swept on delete. reserve_sequence state lives in a path-scoped side table; exact/prefix delete now clears it in the libSQL, Postgres, and in-memory backends so delete/recreate restarts from 1 instead of resuming stale state. Sibling paths sharing a string prefix are not swept. (CodeRabbit major.) - runner_lease_cache is keyed by TurnRunId instead of a stringified key. (CodeRabbit.) - try_write_new_message_transactionally now retries the transaction on an optimistic-concurrency conflict at commit, as the surrounding loop and the "CAS retries exhausted" error always intended (it previously returned the conflict without retrying — a never-loops latent bug). Postgres-only; libSQL/in-memory return Unsupported before the loop body. Regression tests: redact an append-only finalized assistant message and assert reads show the redaction with a single history row; finalize-existing- draft asserts the single-row in-place invariant; delete clears reserved sequences while preserving a string-prefix sibling. Not changed (false positives, verified against the code): - StoredThreadMessageRecord round-trip: #[serde(flatten)] makes it deserialize cleanly as ThreadMessageRecord. - materialize_message_range: list_thread_messages already merges the append log, so append-only messages are not missed. - Cached-heartbeat vs recovery: lease TTL (90s) exceeds the durable refresh interval (30s) plus expiry margin (30s), so a heartbeating run's durable expiry stays ahead of recovery — no premature requeue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb * fix(extensions,host_runtime): handle FilesystemOperation::ReserveSeq in permission matches The new `ReserveSeq` filesystem operation made two exhaustive `FilesystemOperation` permission matches non-exhaustive, breaking the full-workspace build (caught by the Railway preview deploy and the all-features clippy gate; the per-crate local builds didn't reach these crates). `reserve_sequence` mutates the sequence counter, so it maps to `permissions.write` alongside the other record/event-plane writes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb * style(threads): rustfmt filesystem_service append-event read Collapse the `let Some(events) = ... else` binding onto one line per rustfmt. The Formatting CI gate did not run on the review-fix commit (only label workflows fired there), so this slipped through until the post-merge full CI run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb * fix(threads): order thread list by updated_at, stamped at turn boundaries Resolves the CodeRabbit/Gemini findings on `list_threads_for_scope` (and the cross-thread mis-ranking flagged earlier), realigning the filesystem service with the in-memory reference, which already sorts by `updated_at` and stamps it on activity. The native `reserve_sequence` path stopped rewriting the thread record, so `updated_at` went stale and the list fell back to sorting by `max(message.sequence)` — per-thread sequence, i.e. transcript length, not recency. That also forced a full per-thread transcript scan (`list_thread_messages` for every thread) before pagination, an O(N*M) sidebar cost, and silently ranked unreadable histories as oldest. Changes: - Add `touch_thread_updated_at`: a bounded-CAS stamp of `thread.updated_at` called once per turn boundary (inbound accept + finalized assistant append), best-effort under contention (a lost CAS race means a concurrent writer already advanced the stamp — the safe direction). - Sort `list_threads_for_scope` purely by `updated_at`/`created_at` desc with a stable thread_id tie-break; drop the per-thread transcript scan and the `latest_sequence = 0`-on-read-error fallback entirely. - Extend the activity-ordering contract test to pin the distinguishing case: a chattier-but-staler thread must not outrank a quieter, more-recently touched one (fails under the old sequence sort, passes now). Cost is one extra thread-record write per turn (turn boundary, not per token) — negligible against the per-token writes the sequence primitive removed, and invisible to users, who instead get correct recency ordering and a sidebar whose cost scales with thread count, not transcript volume. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb * fix(threads): make post-commit recency touch best-effort; narrow list comment Addresses two CodeRabbit findings on the Option A recency work: - `touch_thread_updated_at` ran with `?` at three after-commit call sites (inbound accept, draft finalize, finalized append). A non-CAS backend error there failed the call *after* the message was already durable, and since `accept_inbound_message` permits requests without an idempotency key, the caller's retry could duplicate the message. Route all three through a new `touch_thread_updated_at_best_effort` that logs and continues — the stamp is advisory once the write has committed. - Narrow the `list_threads_for_scope` comment: activity ordering never scans transcripts, but title derivation still reads the sliced page, so the prior "transcripts are never scanned here" overpromised. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb * fix(threads): index append-only finalized messages; stamp recency on draft finalize Two correctness fixes for the append/finalize paths: - Append-only finalized messages now write the sequence index, not just the log event. `write_new_message` indexes; `append_message_event` did not, so a finalized assistant reply stored append-only was missing from indexed range reads (`list_thread_messages_range`, summaries, compaction) on threads that also had indexed messages — even though full-history and model-context reads (which merge the append log) already saw it. The id resolves through `read_message_versioned`'s append-log fallback. New regression test: `filesystem_store_range_read_includes_append_only_finalized_message`. - `finalize_assistant_message` (the draft -> finalized path) now stamps thread recency via `touch_thread_updated_at_best_effort`, matching `accept_inbound_message` and `append_finalized_assistant_message`. Without it, the draft/update/finalize path left active threads stale in the `updated_at`-sorted sidebar — the user-visible "latest doesn't come to top" symptom for runtimes that stream a draft then finalize. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb * fix(threads): keep existing threads on legacy sequence counter (migration safety) Prevents the native path-local sequence counter from corrupting threads on instances that predate this change. The native `reserve_sequence` counter starts at 1 for any path with no row. An existing thread already has messages at sequences 1..N and `next_sequence = N+1` on its record, but no native counter row — so the first message after deploy would reserve sequence 1, colliding with the existing message and clobbering its sequence-index entry (orphaning it from range and summary reads). Fix: in the thread-store `reserve_sequence`, branch on the already-read `next_sequence`. Threads with `next_sequence > 1` (sequences already assigned under the legacy per-record counter) keep using that counter; only new/empty threads (`next_sequence == 1`, no messages yet) use the native fast path. Because the native path never rewrites `next_sequence`, a native thread's record stays at 1 and deterministically keeps using native, while a pre-existing thread stays on the legacy counter for its whole life — no thread ever switches counters mid-stream. No trait change, new field, or migration scan; existing-instance data is untouched. Regression test `reserve_sequence_resumes_existing_thread_counter_not_native_restart` simulates a pre-existing thread (`next_sequence = 5`, no native row) and asserts the next reservation is 5, not a native restart at 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb * fix(threads): sweep append-log on delete and repair sequence index on retry Address CodeRabbit data-integrity findings on the finalized-assistant append path: - Backend delete now sweeps `root_filesystem_events` / `event_logs` alongside entries and sequences (libSQL, Postgres, in-memory). Without this, deleting and recreating the same thread path would rehydrate stale append-log history (append-only finalized assistant messages live in the event log). - `append_finalized_assistant_message` re-asserts the sequence index before returning an already-finalized message on idempotent retry. If a prior call appended the log event but died before writing the index, the durable message would otherwise stay invisible to indexed range/context reads. `write_new` is idempotent, so this is a no-op on the fully-persisted path and a repair on the partial-failure path. Regression tests: - in-memory delete sweeps co-located and subtree event logs - idempotent retry repairs a missing sequence index (range read sees it) - append-only finalize test now asserts no per-message file was written - finalize-existing-draft test asserts the run index resolves to the single in-place record Skipped CodeRabbit's batch-append-log-fallback suggestion: it is a perf optimization (O(K*M) bounded by append-only messages in a range), not a correctness issue, and a larger change better tracked separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQzj2447pq2BUfgY3BUSJb --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: firat.sertgoz <firatsertgoz@alumni.sabanciuniv.edu>
12 lines
513 B
SQL
12 lines
513 B
SQL
-- Path-local monotonic sequence allocator for row-shaped stores.
|
|
--
|
|
-- `root_filesystem_events` assigns globally increasing ids, which are correct
|
|
-- for event replay cursors but not for per-record-set ordering such as thread
|
|
-- message sequences. This table keeps one atomic counter per virtual path.
|
|
|
|
CREATE TABLE IF NOT EXISTS root_filesystem_sequences (
|
|
path TEXT PRIMARY KEY CHECK (path LIKE '/%'),
|
|
next_seq BIGINT NOT NULL CHECK (next_seq > 0),
|
|
updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
|
|
);
|