Skip to content

Pin live that a forget drops every pack; say recall packs carry stored text - #210

Merged
M3gA-Mind merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:test/live-pack-drop-premise
Oct 6, 2026
Merged

M3gA-Mind merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:test/live-pack-drop-premise

Conversation

@M3gA-Mind

Copy link
Copy Markdown
Collaborator

Summary

Two leftovers from #207–#209, which merged before they were pushed:

  1. A live test of what recall's dropped-pack retry rests on. a_forget_anywhere_drops_every_pack_the_server_holds (Direct) checks against the real server that:

    • a forget in an unrelated scope makes the next use_pack_id answer a 404;
    • a pack built after the forget answers.

    The retry itself runs inside one recall call, where an outside test cannot place a forget between the recall and the answer. So the live suite pins the server behaviour instead, and an_answer_whose_pack_expired_recalls_that_scope_again pins the engine side. If CortexDB stops dropping packs or answers with another status, this test fails on the real server.

  2. Docs: the log module and the test double still said recall prefixes [role] to a pack event's text. CortexDB 0.10.3 and 0.10.4 return the stored text in layers.events; the marker is only in context_block (API §9.4). [stacked on #207] Write events as prose with the envelope in labels; let items opt out of derivation #208 changed the double accordingly; this corrects the prose.

Related issue

Follows #207 (review threads PRRT_kwDOT0GFnc6po1pd, PRRT_kwDOT0GFnc6po1pi, PRRT_kwDOT0GFnc6po1po) and #209 (PRRT_kwDOT0GFnc6ppDvd, PRRT_kwDOT0GFnc6ppDv8).

API or behavior changes

None. A live test and two doc comments.

Validation

Local, on this tree (identical to the verified b9f154f7), in a target dir of this worktree's own:

  • cargo fmt --all -- --check: ok
  • cargo clippy --all-targets --all-features -- -D warnings: ok
  • cargo build --all-targets --all-features: ok
  • cargo test --all-features: ok, 1130 passed
  • cargo test: ok, 442 passed
  • RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features: ok
  • cargo run -p tinymemory-integrations --example basic: ok
  • The CI "Refuse inline test code" script: ok
  • cargo llvm-cov … --fail-under-lines 80: ok, 94.20% lines
  • cargo hack --feature-powerset --depth 2 --workspace check --all-targets: ok
  • scripts/cortexdb-live.sh on a fresh local CortexDB v0.10.4: ok, 5 + 1 + 3 passed; the new test also passed 3/3 on fresh servers.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

@tinysweeper

tinysweeper Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Ready for maintainer review
Priority: medium
Reviewed head: 145e78af21fa
Updated: 1791318199 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 1
Tests 2 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

  • medium · e2e · Record that the new live-server test never ran on this head — The behavioural change (recall pack invalidation on cross-scope forget, as pinned in `engine/recall.rs`) is covered by `a_forget_anywhere_drops_every_pack_the_server_holds`, which (crates/tinymemory\-integrations/tests/live\_cortexdb\.rs:451)

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 3 files; 0 findings. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 3 files; 0 findings. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change is a documentation correction plus a live-server test pinning the forget-drops-all-packs behaviour that the recall retry is built on. The test has real assertions (404 on the stale pack, success on the fresh one) and would fail if the behaviour regressed, so it earns its place. The doc edits remove a claim that `recall` prefixes the speaker to event text; the doubled `EnrichedInfo` in `cortex/testing/log.rs` still produces that prefix, so the new wording there is accurate only as a statement about this crate's own output, not about recall generally, and the module doc does not say which — minor, not blocking. I found no new error path, no new branch, and no claim in the diff without a test behind it: the new doc claim in `log/mod.rs` is corroborated by the existing tests in `cortex/testing/log.rs` (which assert events carry the stored text with no prefix). _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The diff matches the description: it adds one live Direct-only test pinning that a forget in an unrelated scope drops every held pack (404 on the next use_pack_id, success for a fresh pack) and corrects two doc comments that described recall prefixing `[role] ` to event text. Both are accurately summarised, no behaviour change is hidden, and nothing else is touched, so it looks safe to merge. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

e2e

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The pull request adds a live end-to-end test that drives the real CortexDB server over HTTP: it writes to two scopes, builds a recall pack, forgets in the unrelated scope, and asserts the pinned pack answers 404 while a fresh pack answers. That directly exercises the behavioural surface the retry logic in `engine/recall.rs` relies on, and the two doc-only edits change no external surface. The test is gated on `TINYMEMORY_LIVE_CORTEXDB_URL` and the harness header reports no e2e workflow in the tree and no e2e jobs ran on this head, so I could not see the test actually pass in CI — but the test itself is present, self-contained and correctly wired to the existing docker-compose harness via `TINYMEMORY_TEST_CORTEX_KEY`. With no way to observe a run, I record that as unverified rather than assert a defect. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._
  • Evidence: crates/tinymemory\-integrations/tests/live\_cortexdb\.rs — Record that the new live-server test never ran on this head
Evidence and run details
  • Models: gpt-5.6-luna, glm-5.3-flash
  • Spend: $0.001163
  • Tokens: 97921 input · 7263 output · 15288 cached · 0 embedding
Head State Pass summary
145e78af21fa ready for maintainer review 1 active finding(s), 0 resolved finding(s) (at 1791318199)

tinysweeper 0.1.0

@coderabbitai

coderabbitai Bot commented Oct 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 3 billable files and costs up to $0.75.

Or wait 39 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6d9e8e95-dea2-4939-a8cd-6094aed3d21a
📥 Commits

Reviewing files that changed from the base of the PR and between a361dc9 and 145e78a.

📒 Files selected for processing (3)
  • crates/tinymemory-integrations/src/cortex/log/mod.rs
  • crates/tinymemory-integrations/src/cortex/testing/log.rs
  • crates/tinymemory-integrations/tests/live_cortexdb.rs
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

             $0.0012 · 97,921 in / 7,263 out · 15,288 cached (16%) · gpt-5.6-luna, glm-5.3-flash
critique:    $0.0004 · 34,626 in / 2,428 out · 8,132 cached (23%)  · gpt-5.6-luna
security:    $0.0004 · 33,086 in / 1,394 out · 7,156 cached (22%)  · gpt-5.6-luna
tests:       $0.0001 · 12,433 in / 721 out   · 0 cached (0%)       · glm-5.3-flash
description: $0.0001 · 5,606 in  / 94 out    · 0 cached (0%)       · glm-5.3-flash
e2e:         $0.0001 · 7,822 in  / 526 out   · 0 cached (0%)       · glm-5.3-flash

Comment thread crates/tinymemory-integrations/tests/live_cortexdb.rs
@tinysweeper tinysweeper Bot added the priority: p2 Soon. Real but survivable — a rough edge, a gap, a thing that will bite later. label Oct 6, 2026
@M3gA-Mind
M3gA-Mind merged commit 8bd386e into tinyhumansai:main Oct 6, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p2 Soon. Real but survivable — a rough edge, a gap, a thing that will bite later.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant