fix: make the auto-capture watermark survive a restart, and bound it when unknown#951
Draft
gorkem2020 wants to merge 6 commits into
Draft
fix: make the auto-capture watermark survive a restart, and bound it when unknown#951gorkem2020 wants to merge 6 commits into
gorkem2020 wants to merge 6 commits into
Conversation
gorkem2020
added a commit
to gorkem2020/memory-lancedb-pro
that referenced
this pull request
Jul 15, 2026
Checkpoint recut of deploy/fleet-2026-07-15b (21 PRs) surfaced several merge-time regressions and stale test fixtures once the full test/tsc/build gate ran end to end: - index.ts: a plain (non-conflicted) auto-merge reintroduced CRLF into the modelRun:true block inside generateReflectionTextUnbounded; restored to LF. - index.ts: restored PR CortexReach#934's "regex fallback skipped" debug log + return in the below-threshold defer branch, dropped when PR CortexReach#951's merge took the persistence-based rollback wholesale instead of unioning both changes. - index.ts: removed a duplicate minMessages declaration left over from reconciling PR CortexReach#939's assistant-context accumulation block with PR CortexReach#951's own minMessages usage earlier in the same function. - src/admission-control.ts: hoisted the constructed-durable short-circuit from evaluateWithUtility() into evaluate() itself, so a constructed durable candidate is rejected before scoreUtility() spends an LLM call, not just before its score is blended in. - src/admission-control.ts: restored the Grounding/Conversation register fields in buildUtilityPrompt's user template (dropped during the PR CortexReach#938 split-prompt merge), since PR CortexReach#927's own admission tests depend on the judge seeing them inline. - Updated several PR CortexReach#927/CortexReach#938/CortexReach#939 test assertions that predate the split-prompt {system, user} return shape to destructure the right half instead of matching the whole object, and updated stale "## Recent Conversation" / "Evaluate whether this candidate" mock-routing strings to the current prompt text. - test/assistant-context-capture-counting-symmetry.test.mjs: gave each captureAssistant/payloadShape scenario its own directory so the new watermark sidecar file (keyed by dirname(dbPath) + sessionKey) doesn't bleed state between scenarios that previously only shared in-memory state; updated the delta-only sanity check to reflect the documented rollback characteristic where per-turn deltas below minMessages never accumulate. - test/smart-extractor-branches.mjs: updated a fixture comment/payload that had already predicted and documented this exact combination, switching to full accumulated history per turn as prescribed. Full suite (minus the environment-only Ollama port-conflict test), tsc, and build all green after these fixes.
gorkem2020
force-pushed
the
fix/watermark-restart-survival
branch
from
July 17, 2026 21:37
b32b856 to
12bccdd
Compare
autoCaptureSeenTextCount is an in-memory Map, wiped by any plugin/process restart. A session whose progress toward extractMinMessages was banked before a restart (e.g. via the ingress accumulator, where a below-threshold turn's tentative advance is intentionally kept rather than rolled back) loses that progress silently. If the session's agent_end payload is delta-only, there is no way to recover the lost count from the payload itself. Persist a snapshot of the watermark to a small JSON sidecar next to the LanceDB dir (matching the existing .compaction-state.json convention) on every mutation, and rehydrate it once at plugin construction. The sidecar inherits the same AUTO_CAPTURE_MAP_MAX_ENTRIES bound already applied to the in-memory Map, so it cannot grow unbounded. While here: bring the below-threshold "roll the cursor back" behavior (currently live in the deployed build but never committed to any branch) into source control, now routed through the same persisting setter so a restart can't un-defer content that was rolled back pre-restart either. Deliberately narrower in scope than the deployed version: this keeps the counter-rollback semantics but does not also skip the regex fallback for below-threshold turns, since that additionally changes when explicit single-message "remember" commands fire and has its own pre-existing test coverage (test/smart-extractor-branches.mjs) -- out of scope for a watermark/turn-accounting fix. Red-first: test/autocapture-watermark-restart-survival.test.mjs simulates a restart via resetRegistration() + a fresh register() against the same dbPath. Covers an ingress-fed delta-only session resuming from its persisted count and firing once threshold is crossed, a history-carrying session's slice cursor surviving a restart without re-extracting already -consumed history, and the rollback-on-skip mechanism itself (pinned so future watermark work can't silently drop it). test/auto-capture-watermark -store.test.mjs covers the sidecar's load/save edge cases in isolation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rk is unknown Even with a restart-survivable watermark, a genuinely unknown one (first- ever run for a session, or persisted state lost) meeting a history-carrying agent_end payload would still take the unsliced default (newTexts = eligibleTexts) and hand the entire transcript to extraction in one call -- expensive, and floods the extraction with mostly-stale content. Add a third branch alongside the existing ingress and known-cursor slicing: when previousSeenCount is 0 and the eligible history is bigger than one batch, cap the injected window to the most recent extractMinMessages texts (trimmed further against extractMaxChars if even that's too big -- see capUnknownWatermarkWindow in src/auto-capture-cleanup.ts), and advance the watermark straight to the full history length. The older prefix is forfeited, not queued: the next turn is a normal small delta from the new baseline, not a re-read of what was skipped. Red-first: test/auto-capture-unknown-watermark-window.test.mjs covers the capping function in isolation (count cap, char-budget trim, single-text- exceeds-budget edge case). test/autocapture-unknown-watermark-injection .test.mjs covers the full hook: a 40-turn history caps to the batch window and forfeits the rest, the watermark lands at the full 40 (not the 2-text window), the next turn is a normal delta, and a small under-threshold history is left untouched (not a false-positive trigger). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ited INFO line The one debug line that reveals whether a session's agent_end payload is delta-only or history-carrying was only written at DEBUG, invisible in the file logger used in production. Diagnosing a stuck watermark meant reconstructing this from indirect evidence across hours of unrelated log lines. Add a compact INFO-level line, emitted once per session per process (a new autoCapturePayloadShapeLoggedSessions Set, deliberately NOT persisted -- a restart re-emitting it is the point, not a gap): messages, eligible, previousSeen, cumulative, and fired (cumulative >= minMessages). One line per session per process, not one per turn, so it can't become its own kind of log spam. Red-first: test/autocapture-payload-shape-observability.test.mjs covers the first-turn line and its exact fields, silence on later turns in the same process, a fresh line for a different session (rate limit is per-session), a fresh line again after a simulated restart (rate limit is per-process, not persisted), and fired=yes once cumulative crosses minMessages. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-injection + observability changes Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
gorkem2020
force-pushed
the
fix/watermark-restart-survival
branch
from
July 18, 2026 15:52
12bccdd to
223813d
Compare
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
1 participant
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.
Problem
autoCaptureSeenTextCountis an in-memory Map, wiped by any plugin or host process restart. A session that had banked partial progress towardextractMinMessagesbefore a restart (for example through the ingress accumulator, where a below-threshold turn's tentative advance is intentionally kept rather than rolled back) silently loses that progress. If the session'sagent_endpayload is delta-only (only the newest turn, not the full transcript), there is no way to recover the lost count from the payload itself, so extraction can stall indefinitely for that session.Separately, even with a durable watermark, a genuinely unknown one (first-ever run for a session, or persisted state that was wiped) meeting a history-carrying payload would still hand the entire transcript to extraction in one call.
And the one log line that would let anyone tell whether a given session's payload is delta-only or history-carrying was DEBUG-only, invisible in the file logger used in production.
What changed
Restart survivability. The watermark is now persisted to a small JSON sidecar next to the LanceDB directory (matching the existing
.compaction-state.jsonconvention) on every mutation, and rehydrated once at plugin construction. The sidecar inherits the same bound already applied to the in-memory Map, so it cannot grow unbounded.While in this code path, the below-threshold "roll the cursor back" behavior (already live in the deployed build, but never actually committed to any branch, so it did not previously exist in git history) was brought into source control, now routed through the same persisting setter so a restart cannot un-defer content that was rolled back pre-restart either. This PR is deliberately narrower than the deployed version: it keeps the counter-rollback semantics but does not also skip the regex fallback for below-threshold turns, since that additionally changes when explicit single-message "remember" commands fire and has its own pre-existing test coverage.
Bounded injection for an unknown watermark. When the watermark is genuinely unknown (
previousSeenCount === 0) and the eligible history is larger than one batch, the injected window is capped to the most recentextractMinMessagestexts (trimmed further againstextractMaxCharsif even that is too big), and the watermark jumps straight to the full history length. The older prefix is forfeited, not queued: the next turn is a normal small delta from the new baseline.Observability. A compact INFO-level line is now emitted once per session per process (not once per turn):
messages=N, eligible=N, previousSeen=N, cumulative=N, fired=yes/no. This single line answers the delta-only-vs-history-carrying question that previously required reconstructing indirect evidence across hours of DEBUG-only logs.Relationship to other in-flight work
Builds on the counter-reset fix from #932. Related to the still-open gap in #948 (the valid-empty extraction path's watermark reset is unconditional where the success path is not); that is being fixed directly on #948's own branch rather than here, to keep this PR scoped to restart-survivability, bounded injection, and observability.
Tests
Red-first throughout:
test/autocapture-watermark-restart-survival.test.mjs: an ingress-fed delta-only session resumes from its persisted count and fires once threshold is crossed after a simulated restart; a history-carrying session's slice cursor survives a restart without re-extracting already-consumed history; the rollback-on-skip mechanism itself is pinned so future watermark work cannot silently drop it.test/auto-capture-watermark-store.test.mjs: the sidecar's load/save edge cases in isolation (missing file, malformed JSON, non-numeric entries, round-trip, directory creation, unwritable parent).test/auto-capture-unknown-watermark-window.test.mjsandtest/autocapture-unknown-watermark-injection.test.mjs: the capping function in isolation, plus the full hook proving a 40-turn history caps correctly, the watermark lands at the full length, and the next turn is a normal delta.test/autocapture-payload-shape-observability.test.mjs: the new INFO line's fields, its once-per-session-per-process rate limit, and that a restart re-arms it.Full suite (minus a pre-existing, environment-specific Ollama-port test unrelated to this change) plus
tscandbuildare green. New test files are registered in bothpackage.json's test chain andscripts/ci-test-manifest.mjs.Fixtures throughout are entirely synthetic; no real fleet data.