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. |
Round two of the contract surface openhuman#5560 needs before the host can stop linking tinycortex and tinymemory-core. Round one (tinyhumansai#122, v1.13.5) left the tree_runtime operation set and flavour.rs as blocked residue; these are the doors that clear it. METHODS goes 131 -> 138. The six runtime members are the markdown time tree addressed node by node, which is what the host's tree_summarizer_* RPCs report verbatim: RuntimeBufferWrite buffer content, answering with the landing path RuntimeReadNode one node, or None RuntimeReadChildren a node's direct children RuntimeTreeStatus one namespace's shape and coverage RuntimeSummarize drain the buffer, answering with the hour node written RuntimeRebuild rebuild the tree from its hour leaves Append, DrillDown, Seal and Cascade are the same tree at a coarser grain, and each folds away a piece of the reply those RPCs carry — migrating onto them would have changed the host's wire format, and a door that changes what the host reports is not a door but a new surface. FlavourProfile collapses the whole compiled-root lookup the host ran against the engine directly behind one scope-shaped question. The contract goes to (4, 0). All seven land on Tree, a family a driver may already advertise, and version.rs is explicit that such an addition cannot be made minor-safe: negotiation is family-granular, not method-granular, so there is no way to advertise "Tree, but without the new methods" and an older driver still advertising Tree would be bound and then asked for a method it has never heard of. The major half refuses that bind up front instead of discovering it at the call. is_compatible compares the major half only, so this refuses the bind against every deployed v1.13.5 module and hosts must re-pin; the openhuman host re-pins in openhuman#5875. Round one did not bump, and neither did tinyhumansai#85/tinyhumansai#86/tinyhumansai#89/tinyhumansai#90 — version_tests.rs already calls that drift rather than precedent, and its history note now records tinyhumansai#122 alongside them. Every trait method is defaulted to unsupported(Tree), so a driver built against the older contract keeps compiling. The two provider-backed members resolve the summariser before the engine is asked anything: these are a person's explicit "run now", and a runner that could not have run must say so rather than answer None as if it had looked. Seal and Cascade keep their empty short-circuits; they are the scheduler's. The three members answering with tree nodes are checked against the response ceiling. A level's max_tokens bounds a node's summary and nothing else — the fold applies it to the body alone — while metadata carries a pending-fold receipt naming every buffer file the pass drained, so it grows with how much was buffered rather than with any budget. Without the check an oversized node fails during frame encoding; with it the caller gets BUDGET_EXCEEDED and a reason. The pre-existing tree members that skip the check are left alone. The module's seven service members are covered in pairs — the refusal a driver without the Tree family must give, and the answers the port carries back from one that has it. Without them the module's production-source coverage gate fell to 77.4%: the seven delegations are only reachable through the loader E2E, which is `#[ignore]`d and so invisible to llvm-cov, and v1.13.5 had just 0.33 points of headroom over the 80% floor. The tinymemory-bus README's member count was still the hardcoded 120 that round one replaced with METHODS.len() in the crate docs but missed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.