Repository navigation
[stacked on #208] Key each event by its own body; skip the lookup on the turn hot path - #209
Conversation
…udget beliefs - Recall builds the chosen scope's pack again when /v1/answer answers 404 for use_pack_id, up to three answers. CortexDB 0.10.4 drops every pack it holds on any successful forget, even in another scope, so a concurrent forget made recall fail (CortexDB live at 9d40d5d). - get and list return a chunked document whole only when every piece agrees on one positive count, each index is below it, and all are present; an unchunked envelope of the same id is the whole body and wins over pieces. - The beliefs read sends max_tokens for each belief it asks for. - The live long-document test is two pieces, so its forget stays inside the request timeout, and after forget it polls fetch until no piece of the item is left. - The spec states the chunking threshold on the envelope as a piece.
- A 404 for use_pack_id now repeats the whole round: every scope's pack is built again and the answer asked from the new chosen pack, so the citations also come from packs read after the drop and cannot cite an item forgotten in between. At most three rounds. - docs/architecture/cortex-wire.md was over the 500-line limit; its "Chunked documents" section is now docs/architecture/cortex-chunks.md.
|
Warning Review limit reached
This review includes 30 billable files and costs up to $7.50. Or wait 53 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (30)
Comment |
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 16 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Changes requested Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Resolved this pass
Before merge
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0132 · 1,109,361 in / 61,380 out · 242,050 cached (22%) · gpt-5.6-luna, glm-5.3-flash, gpt-6-luna
critique: $0.0071 · 588,828 in / 39,193 out · 125,311 cached (21%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0035 · 334,896 in / 14,815 out · 89,859 cached (27%) · gpt-5.6-luna
tests: $0.0005 · 73,525 in / 1,060 out · 26,880 cached (37%) · glm-5.3-flash
description: $0.0004 · 35,820 in / 1,063 out · 0 cached (0%) · glm-5.3-flash
e2e: $0.0004 · 38,137 in / 1,846 out · 0 cached (0%) · glm-5.3-flash
Run together, one live test's forgets drop the packs another is about to answer from, and load the server enough that forgetting a ~700 KiB document (seconds per ~240 KiB event on CortexDB 0.10.4) outlasted the request timeout. Each live test now holds one async lock for its run, and the long document is back to 24 pages (three pieces, two boundaries).
…of derivation CortexDB extracts from an event's text and splits it for search at sentence boundaries, which a JSON text lacks, and counts that text toward its 1 MiB limit; labels are its app-metadata extension point. So events are now written as v3: - content.text is the item's own text: the body or piece, the turn's text, or the learning's statement; - context.labels hold the lookup labels, readable kind:/file:/page:/ section: labels (at most 256 bytes each, never lang:), and the rest of the envelope as compact JSON in tm:e:<NN>: parts of at most 240 bytes. An event with empty text, or whose labels would pass 64, is written as v2 (the whole envelope as JSON text) as every event was before. Readers take both, so existing stores need no rewrite. A recovered hosted write is matched on its labels as well as its text, since two v3 turns can say the same words. MemoryMeta gains derive: Option<bool>. Some(false) stores and indexes an item but asks CortexDB to derive nothing from it (directives.extract: []); every tool turn is sent the same way. Unset, it is not serialized, so fingerprints are unchanged. The test double's recall no longer prefixes a pack event's text with [role]: CortexDB 0.10.3 and 0.10.4 return the stored text there.
CortexDB 0.10.4 (measured): a reused body idempotency_key with the same body is a replay answered with the first event's id and replayed_from_idempotency: true; with another body it is a 409; and /v1/forget by memory_ids releases the key. The old reason for fresh keys (a forgotten item's key is never released) no longer holds. - Each event's idempotency_key is tm3: and 56 hex of the SHA-256 of its request without the key (60 chars, under CortexDB's 64). An identical retry replays; any change to the body is a new key, so a 409 for a reused key cannot happen. - Writes read replayed_from_idempotency (single, bulk, hosted); a receipt is a replay when every written event was replayed. - The pre-write lookup is skipped only for a Direct, accepted-only store of one single-turn conversation (the agent lifecycle's two writes per turn). Keys last 24 hours and change with observed_at, so documents, batches, waited-for writes and every hosted write still look up. - The hosted Idempotency-Key claim stays fresh per write: the hosted API answers every replay of a claim with 409. - The test double releases a key on forget, as 0.10.4 does.
- live_cortexdb: a turn logged twice on the hot path is written once, the retry answered by CortexDB as a replay of the first event. - The key is the first 56 hex digits (224 bits) of the SHA-256 of the compact JSON body; a body-derived key never produces the 409 for a reused key. - The double's scope-wide forget keeps keys held, as CortexDB's redact-only scope forget does; a forget by memory_ids releases them.
05c6cc7 to
6e1e0f9
Compare
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0101 · 461,448 in / 29,588 out · 31,499 cached (7%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0031 · 253,739 in / 20,409 out · 24,343 cached (10%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0007 · 53,274 in / 2,631 out · 7,156 cached (13%) · gpt-5.6-luna
tests: $0.0027 · 36,973 in / 2,179 out · 0 cached (0%) · glm-5.3-flash
description: $0.0004 · 37,410 in / 533 out · 0 cached (0%) · glm-5.3-flash
e2e: $0.0004 · 40,357 in / 709 out · 0 cached (0%) · glm-5.3-flash
Summary
Each CortexDB event is now keyed by its own body, so an identical retry is a replay CortexDB answers without writing. The pre-write lookup is dropped on the one path where it costs the most: logging a turn.
Stacked on #208, which is stacked on #207. Merge order: #207, #208, then this PR. Its own change is the last commit.
Until now every write minted a fresh
idempotency_key. The stated reason was "CortexDB never releases a key on forget, so a content key would make re-storing a forgotten item a silent no-op". That came from the v1 adapter's notes, on a build that was not recorded. The CortexDB team says otherwise, and CortexDB 0.10.4 agrees (measured with curl on the live harness):202, the first event's id,replayed_from_idempotency: true; nothing written409 IDEMPOTENCY_CONFLICT(withexisting_event_id)/v1/forgetbymemory_ids, then the same key and body422 INVALID_ENVELOPE: "idempotency_key exceeds 64 chars"replayed_from_idempotency: trueRelated issue
None. This is TM-7 from the CortexDB scoping review.
API or behavior changes
No public API change. Behaviour:
tm3:plus the first 56 hex digits (224 bits) of the SHA-256 of the compact JSON request body without its key: 60 characters, under the limit of 64.observed_aton an unchanged item, a different envelope layout) is a new key and a new event, so the 409 for a reused key cannot happen.tm3names the event layout.replayed_from_idempotencyon single, bulk and hosted answers; absent counts asfalse. A receipt isreplayedwhen nothing was due, or when CortexDB replayed every event written.observed_at, so it cannot replace the item lookup that makes an unchanged re-synced file a replay. The lookup is skipped only for a Direct,WaitFor::Acceptedstore of one single-turn conversation. That is the agent lifecycle's two writes per turn, where a retry is the replay that matters and a listing per write costs turn latency. Documents (brain ingest, sources), batches, waited-for writes and every hosted write still look up.Idempotency-Keyclaim stays fresh per write, reused across that write's retries only. The hosted API answers every replay of a claim with 409.lose_last).Validation
Local, at 6e1e0f9, in a target dir of this worktree's own:
cargo fmt --all -- --check: okcargo clippy --all-targets --all-features -- -D warnings: okcargo build --all-targets --all-features: okcargo test --all-features: ok, 1126 passedcargo test: ok, 438 passedRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features: okcargo run -p tinymemory-integrations --example basic: okcargo llvm-cov … --fail-under-lines 80: ok, 94.16% linescargo hack --feature-powerset --depth 2 --workspace check --all-targets: okscripts/cortexdb-live.sh's three suites on a fresh local CortexDB v0.10.4: oklive_cortexdbandlive_cortex_lifecycleon 3 fresh servers: 3/3 eachRevert-checks (each made its test fail, then restored):
a_logged_turn_skips_the_lookup_and_a_retry_is_a_replayfails;every_other_store_still_looks_its_items_up_firstfails;replayed_from_idempotency:a_logged_turn_skips_…fails (the retry is not reported as a replay);a_logged_turn_skips_…fails (the retry writes a second event).Tests
a_logged_turn_sent_twice_is_written_oncelogs a turn twice on the hot path. The retry isreplayedwith the same id, and one conversation is listed. A revert-check that ignoresreplayed_from_idempotencyfails it against the real server, so the server sends the flag on the single-event path.tm3:and at most 64 characters. The same body gives the same key; a differentobserved_atgives a different key.an_item_forgotten_and_stored_again_is_written_againnow passes because forget releases the key, as on 0.10.4.Documentation
docs/architecture/cortex-flows.md(store: replay detection and body keys);docs/architecture/cortex-wire.md(the request body, CortexDB behaviours);docs/architecture/testing.md(the double);crates/tinymemory-integrations/src/cortex/README.md;log,engine/store,transport).Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the description