fix(remote): skip records a hosted import finds already held - #184
YellowSnnowmann merged 1 commit into
Conversation
A hosted import appended every record, and an append is a new version even
when the record is already there. A copy run again therefore wrote every
record twice: on staging a re-run of Copy & switch took about nine minutes
and left 600 events behind 300 Gmail keys. The migrate driver documents the
opposite ("rerunning is safe because targets skip records they recognise").
Before a batch first writes to a namespace it now reads what the namespace
holds, once, with the fold export reads. A record held with the same
content, category, session and taint is counted as skipped and not written;
any difference is written as before. The batch counts what it wrote as held,
so a record repeated in one batch is written once. The read rides out a
busy backend with the same pauses as a write. A first copy pays one listing
per namespace in each batch, which answers empty.
Tiny Sweeper reviewTiny 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: Incomplete Review snapshot
Completeness: Incomplete 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
Pending checks: AgentMemory E2E, CortexDB full simulation Could not review: crates/tinymemory-remote/src/cortex_provider/portability.rs, crates/tinymemory-remote/src/hosted_test.rs Before merge
How this fits togetherflowchart LR
n0["...ts_once_per_scope_and_never_probes_recall<br/>changed"]:::changed
n1["...waits_out_a_slow_listing_and_a_rate_limit<br/>changed"]:::changed
n2["provider"]:::impacted
n3["lock"]:::impacted
n4["iter"]:::impacted
n5["assert"]:::impacted
n6["..._maps_to_a_memory_path_and_never_a_v1_one"]:::impacted
n7["...tch_falls_back_to_ordered_per_item_writes"]:::impacted
n0 -->|calls| n2
n0 -->|tests| n2
n1 -->|calls| n2
n1 -->|tests| n2
n6 -->|calls| n2
n6 -->|tests| n2
n6 -->|calls| n3
n6 -->|tests| n3
n6 -->|calls| n4
n6 -->|tests| n4
n6 -->|calls| n5
n7 -->|calls| n2
n7 -->|tests| n2
n7 -->|calls| n3
n7 -->|tests| n3
n7 -->|calls| n4
n7 -->|tests| n4
n7 -->|calls| n5
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID:
Comment |
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinymemory-remote/src/cortex_provider/portability.rs, crates/tinymemory-remote/src/hosted_test.rs.
$0.0031 · 48,698 in / 7,820 out · 2,560 cached (5%) · ladder/vectors, deepseek/deepseek-v4-flash · 537 embedded
tests: $0.0008 · 15,829 in / 2,033 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0004 · 7,634 in / 1,340 out · 2,048 cached (27%) · deepseek/deepseek-v4-flash
e2e: $0.0009 · 19,583 in / 1,517 out · 0 cached (0%) · deepseek/deepseek-v4-flash
| /// A record the backend refuses (a 400-class answer, or a namespace that | ||
| /// cannot be a hosted scope) is counted in [`ImportOutcome::failed`] with a | ||
| /// reason that names the record and the backend's code, never its content. | ||
| /// A record the namespace already holds with the same content, category, |
There was a problem hiding this comment.
Add an end-to-end test for the skip-on-unchanged import behaviour
The change introduces a new behaviour: import_records reads what the namespace holds before writing and skips records whose content, category, session and taint are unchanged. This is a meaningful behavioural change with external surface (the public import_records API). It is covered only by unit tests in hosted_test.rs. No end-to-end test in the repository (module_e2e.rs, server_e2e.rs, integration scripts) exercises this skip path. The conformance suite's assert_export_import_round_trip does not test re-importing identical records. Add an end-to-end test that drives the memory provider as a client and verifies that re-importing the same records produces skipped counts and that changes to any of the four fields force a write.
[RULE] e2e-uncovered ·
v1.22.2 carries two hosted fixes found testing on staging: tinyhumansai/tinymemory#183 recalls each kind of synced source on its own, so a search across synced content follows the question, and tinyhumansai/tinymemory#184 skips records a hosted import finds already held, so Copy & switch run again writes only what changed. Registry record (version, release URL, the 11 digests from the release's checksum.toml), ARTIFACT_CAPABILITIES_PIN and its re-read note, the 6 CI pins, the vendor gitlink on the tag, and both lockfiles. The capability files are unchanged since v1.21.1, so the advertised families stay the same.
v1.22.2 carries two hosted fixes found testing on staging: tinyhumansai/tinymemory#183 recalls each kind of synced source on its own, so a search across synced content follows the question, and tinyhumansai/tinymemory#184 skips records a hosted import finds already held, so Copy & switch run again writes only what changed. Registry record (version, release URL, the 11 digests from the release's checksum.toml), ARTIFACT_CAPABILITIES_PIN and its re-read note, the 6 CI pins, the vendor gitlink on the tag, and both lockfiles. The capability files are unchanged since v1.21.1, so the advertised families stay the same.
Summary
A hosted import appended every record, and on this wire an append is a new version even when the record is already there. A copy run again therefore wrote every record a second time.
On staging, re-running Copy & switch:
The migrate driver already promises the opposite: "rerunning is safe because targets skip records they recognise".
Now, before a batch first writes to a namespace, it reads what the namespace holds once, using the same fold export uses.
ImportOutcome::skippedand not written.import_patience).Related issue
Part of tinyhumansai/openhuman#6718 (hosted CortexDB as a memory engine), found in end-to-end testing on staging.
API or behavior changes
No public API change.
Behavior, hosted wire only:
import_recordsnow fillsskippedfor records the target already holds unchanged, instead of writing them again. A re-run costs a few listings instead of one write per record.Existing duplicates are not removed by this change; it only stops new ones.
Validation
Commands actually run, with their outcome:
cargo fmt --all -- --check: cleancargo clippy --all-targets --all-features -- -D warnings: cleancargo build --all-targets --all-features: okcargo test --all-features: 2394 passed, 0 failedTests
New tests:
a_hosted_import_run_again_writes_only_what_changed:a_record_repeated_in_one_batch_is_written_once.a_hosted_import_rides_out_a_rate_limited_read: one pause fails the batch before any write; two pauses ride out seven 429s.Changed:
a_hosted_import_waits_once_per_scope_and_never_probes_recallnow expects two listings, one read and one visibility wait. It still expects no recall.Mutation-checked. Each of these fails at least one test:
Documentation
The module docs and
hosted_import_recordsdocs describe the skip and its cost.Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the description