Skip to content

fix(recall): preserve immutable capture hints and retained bytes - #4542

Open
yeahjack wants to merge 24 commits into
rtk-ai:developfrom
yeahjack:fix/immutable-recall-captures
Open

yeahjack wants to merge 24 commits into
rtk-ai:developfrom
yeahjack:fix/immutable-recall-captures

Conversation

@yeahjack

@yeahjack yeahjack commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Stack / review order

Depends on #4541 (85fe79b282bd41097ae2f8b4ac93d63011acf194), which adds invocation/capture IDs and actual recall-output accounting. This PR targets develop because the dependency branch is in a fork, so GitHub's full diff currently includes #4541. The stable-reference changes are the commits after that dependency in this stack. Please review/merge the dependency first, then rebase this PR; this is not an independent diff.

Problem and reproduction

A recovery hint currently names hash(command slug + original output). Repeating the same bytes overwrites the offset and metadata associated with an earlier hint. With finite storage caps, it can replace the saved bytes too.

Two synthetic regressions fail on the base:

  • Capture first\nsecond\nthird\nfourth\n with first-hidden line 2, then capture it with first-hidden line 4. The original hint returns only fourth, instead of second, third, and fourth.
  • Capture the same output with a 13-byte cap and first-hidden line 2, then a 7-byte cap and first-hidden line 4. The original saved second\nthird\n becomes fourth\n.

Changes

  • Print exact c_<32-hex capture ID> references, reusing feat(tracking): link hook invocations and measure recall output #4541's random capture identity and invocation ownership.
  • Snapshot each capture's offset, command, cwd, exit status, original byte count, saved line count, and truncation state. Existing accounting-only capture rows retain unknown metadata; migration does not invent an old snapshot from a mutable legacy row.
  • Deduplicate actual retained bytes, after applying the cap, under separate internal p_<full SHA-256> payload keys. The hash includes the command slug to preserve existing per-command aggregate-stat semantics. Payload bytes and codec are immutable; only retention time refreshes.
  • Insert payload and capture atomically. Prune orphan references before insertion and after eviction, so maintenance eviction followed by identical output cannot resurrect an old capture.
  • Preserve existing legacy hashes and unique-prefix lookup. New capture IDs require an exact match. --list prints usable capture references and retained legacy entries.
  • Attribute emitted recall bytes to the immutable source capture/invocation; legacy hash reads remain unassigned. A retained ID does not imply its command-history row still exists or that end-to-end attribution is complete.

Expired captures fail clearly rather than returning another run's output. Existing entry/retention limits and the bounded 1,000 capture-record limit still apply. --full returns all bytes saved for that capture, subject to its original storage cap.

This does not widen or migrate the legacy hash namespace, claim to recover already-overwritten legacy data, or supersede #4464's legacy-hash collision fix.

Validation

  • Reproduced both original failures before implementation.
  • cargo fmt --all, cargo clippy --all-targets, cargo test --all: 4,399 passed, 8 ignored across 38 test binaries.
  • cargo doc --no-deps and git diff --check.
  • Added unit coverage for offset/capped-byte stability, immutable cwd/exit/invocation metadata, shared bytes across capped/uncapped captures, codec deduplication, exact references, transaction rollback, legacy migration (including 12 concurrent first-openers), entry eviction, and maintenance-eviction recapture.
  • Added an actual CLI regression: a printed grep hint retains its original 29 hidden lines after recapture, both reads preserve source attribution and actual byte accounting, --list exposes both capture references, and eviction/recapture leaves the old hint expired.

Related storage work: #4537 (byte budget/GC) and #4540 (batch writes). Both use the same recall payload table; their transaction changes need reconciliation when integrating the stack. Because this PR retains the first payload encoding, #4537's test expecting a same-payload gzip-to-raw recapture to replace the encoded size must be updated to expect the retained size.

Release spotcheck against the exact #4541 parent: 60 alternating warmed duplicate-grep captures (30 synthetic lines, --max 1, separate stores) measured median 4.395 ms before and 4.511 ms after; p95 was 6.194 / 6.174 ms. These are noisy shared-cloud measurements, not a claimed speedup or comprehensive performance result.

@rtk-wshm-sync-bot

Copy link
Copy Markdown

wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type bug-fix
🔴 Risk high

Summary

This PR fixes a bug in RTK's SQLite recall store where repeating identical command output with different truncation hints would overwrite an earlier hint's offset, metadata, and retained bytes. It introduces immutable c_<capture_id> references, per-capture metadata snapshots, deduplicated p_<sha256> payload keys, and invocation/retrieval accounting, while preserving legacy hash lookups.

Review Checklist

  • Tests present
  • Breaking change
  • Docs updated

Analyzed automatically by wshm · This is an automated analysis, not a human review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant