Uh oh!
There was an error while loading. Please reload this page.
Answer the backfill flag through the contract, at the scope it actually has - #89
Conversation
…ly has `backfill_in_progress` was briefly a `QueueStats` field in tinyhumansai#86 and was dropped there for a good reason: it is a process-global, while `open_store` serves one store per memory subtree in a single process, so a per-store snapshot carrying it promised a scope it does not have. Leaving the host to call the global directly answers that objection and creates the other half of the same problem. Once re-embedding runs in the module — which is what `Maintenance::reembed` now does — the host-linked static is not merely coarse, it is always `false`, because a `cdylib` has its own statics. A frontend poll deciding whether to keep a re-embed modal open then closes it during exactly the gap the flag exists to cover. Both objections are satisfied by the same shape, which is neither of the two tried so far: a standalone `Maintenance` member, documented as driver-process- wide and explicitly not store-scoped. The honest scope lives in the signature instead of being implied away by the struct it sits in, and the answer comes from the bound driver, which is where the backfill actually runs. It stays undivable from the counts: a backfill re-enqueues itself, so between one link settling and the next being written there is an instant with nothing ready, nothing running and the work unfinished. The conformance test pins that instant — flag up, both counts zero — and restores the global on drop so a failing assertion cannot leak into the rest of the process. Registered in all four lists: the interface block, the module manifest, `tinymemory_bus::METHODS` with its length, and the loader's `EXPECTED_METHODS`. `every_declared_method_is_actually_routed` passes against the built cdylib. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Warning Your free Security trial is over. An organization admin can activate billing to continue. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
How this change flows3 changed behaviours across 17 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 31 further behaviours left out to keep the diagram readable. flowchart LR
n0["MemoryService<br/>changed"]:::changed
n1["...embers_are_exactly_the_published_contract<br/>changed"]:::changed
n2["...s_parked_work_back_to_ready_and_counts_it<br/>changed"]:::changed
n3["...nt_opens_reuse_the_registered_object_path"]:::impacted
n4["...ed_and_only_success_counts_toward_the_cap"]:::impacted
n5["...e_cap_is_reached_through_successful_opens"]:::impacted
n6["open_store"]:::impacted
n7["test_provider"]:::impacted
n8["provider_config"]:::impacted
n1 -->|uses| n0
n2 -->|calls| n8
n3 -->|uses| n0
n3 -->|calls| n6
n3 -->|tests| n6
n3 -->|calls| n7
n3 -->|tests| n7
n4 -->|uses| n0
n4 -->|calls| n6
n4 -->|tests| n6
n4 -->|calls| n7
n4 -->|tests| n7
n5 -->|uses| n0
n5 -->|calls| n6
n5 -->|tests| n6
n5 -->|calls| n7
n5 -->|tests| n7
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
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
Uh oh!
There was an error while loading. Please reload this page.
Summary
Adds
MemoryMaintenance::backfill_in_progress— a standalone contract member, documented as driver-process-wide and explicitly not store-scoped.This is the third shape tried for the same signal, and it is the one that satisfies both objections raised against the first two.
Why not the two shapes already tried
As a
QueueStatsfield (#86, dropped in f0de879). Review objected correctly: it is a process-global whileopen_storeserves one store per memory subtree in a single process.queue_statsis asked of one bound provider for one store, so a global sitting inside that snapshot reads as per-store, and the caller has no way to find out otherwise. A global behind a signature that says so is coarse; a global behind one that does not is wrong.Left to the host calling the global directly (what
maindoes today). That answers the scope objection and creates the other half of the same problem, which Codex caught reviewing tinyhumansai/openhuman#5693:A
cdylibhas its own statics. Once re-embedding runs in the module, the host-linked flag is not merely coarse — it is always false, and the frontend poll closes the re-embed modal during exactly the gap the flag exists to cover.A standalone member fixes both: the scope is stated in the signature rather than implied away by the struct it sits in, and the answer comes from the bound driver, which is where the backfill actually runs.
Why it is not derivable from the counts
A backfill runs as a chain that re-enqueues itself. Between one link settling and the next being written there is an instant with nothing ready, nothing running, and the work unfinished. The consumer is absence-reasoning — deciding whether an empty semantic recall means "nothing remembered" or "not embedded yet" — and it gets that wrong at exactly that instant without this.
The conformance test pins that instant: flag up,
readyandrunningboth zero.Shape
Ok(false)like itsMaintenancesiblings — a driver that never backfills is not backfilling, which is true of it rather than a refusal.tinymemory_core::queue::backfill_in_progress(). Noblocking: it is an atomic load, not a query.Droprather than at the end of the body — a failing assertion must not leak into whatever runs next in the process.Registration
All four lists, which is what #86 learned the hard way (the compiler checks only the array length):
#[tinybus::interface]block intinymemory-module/src/service/mod.rsmethodsintinymemory-module/src/lib.rstinymemory_bus::METHODS, 93 → 94, and the crate doc that names the countEXPECTED_METHODSintests/module_e2e.rsValidation
cargo fmt --all -- --check— both workspacescargo clippy --all-targets --all-features -- -D warnings— root; module workspace--all-targets -- -D warningscargo test -p tinymemory-tinycortex --test full_provider_conformance— 13 passed, including the newthe_backfill_flag_is_reported_through_the_contractcargo test --manifest-path crates/tinymemory-module/Cargo.toml --lib— 49 passed, includingthe_served_members_are_exactly_the_published_contractdlopen'ed release cdylib, one process per test:every_declared_method_is_actually_routed,the_manifest_declares_every_method_the_module_serves,the_module_advertises_the_complete_tinymemory_api,query_and_maintenance_families_dispatch_typed_requests— all passNo dependency changes, so neither lockfile moves.
Release ordering
This must ride the same release as #85 and #86. Those merged after v1.2.0 was cut, so no published artifact implements
store_stats,queue_stats,latest_queue_failureorretry_failed—git tag --containson both merge commits returns nothing. tinyhumansai/openhuman#5693 is blocked on that release and re-pinsmodules/registry.rsto it; adding this member afterwards would cost a second release and a second re-pin.