Skip to content

Answer the backfill flag through the contract, at the scope it actually has - #89

Merged
YellowSnnowmann merged 1 commit into
tinyhumansai:mainfrom
YellowSnnowmann:feat/5560-backfill-in-progress
Aug 24, 2026
Merged

Answer the backfill flag through the contract, at the scope it actually has#89
YellowSnnowmann merged 1 commit into
tinyhumansai:mainfrom
YellowSnnowmann:feat/5560-backfill-in-progress

Conversation

@YellowSnnowmann

Copy link
Copy Markdown
Contributor

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 QueueStats field (#86, dropped in f0de879). Review objected correctly: it is a process-global while open_store serves one store per memory subtree in a single process. queue_stats is 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 main does today). That answers the scope objection and creates the other half of the same problem, which Codex caught reviewing tinyhumansai/openhuman#5693:

After the settings/login paths now start re-embedding through Maintenance::reembed, the backfill state belongs to the separately compiled TinyMemory module, but backfill_status_rpc still reads tinymemory_core::queue::backfill_in_progress() from the host-linked copy.

A cdylib has 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, ready and running both zero.

Shape

  • Defaulted to Ok(false) like its Maintenance siblings — a driver that never backfills is not backfilling, which is true of it rather than a refusal.
  • The TinyCortex driver reads tinymemory_core::queue::backfill_in_progress(). No blocking: it is an atomic load, not a query.
  • The test sets a process-global, so it restores it on Drop rather 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):

  • the #[tinybus::interface] block in tinymemory-module/src/service/mod.rs
  • the module manifest's methods in tinymemory-module/src/lib.rs
  • tinymemory_bus::METHODS, 93 → 94, and the crate doc that names the count
  • the loader's EXPECTED_METHODS in tests/module_e2e.rs

Validation

  • cargo fmt --all -- --check — both workspaces
  • cargo clippy --all-targets --all-features -- -D warnings — root; module workspace --all-targets -- -D warnings
  • cargo test -p tinymemory-tinycortex --test full_provider_conformance — 13 passed, including the new the_backfill_flag_is_reported_through_the_contract
  • cargo test --manifest-path crates/tinymemory-module/Cargo.toml --lib — 49 passed, including the_served_members_are_exactly_the_published_contract
  • Loader E2E against the real dlopen'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 pass

No 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_failure or retry_failedgit tag --contains on both merge commits returns nothing. tinyhumansai/openhuman#5693 is blocked on that release and re-pins modules/registry.rs to it; adding this member afterwards would cost a second release and a second re-pin.

…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>
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c322c583-262b-49bb-be05-9ac8e54777c5

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.

❤️ Share

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

@tinysweeper

tinysweeper Bot commented Aug 24, 2026

Copy link
Copy Markdown

How this change flows

3 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
Loading

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.

tinysweeper 0.1.0

@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.0000 · 0 in / 0 out · 439 embedded · openrouter/openai/text-embedding-3-small

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Aug 24, 2026
@YellowSnnowmann
YellowSnnowmann merged commit 81dba6d into tinyhumansai:main Aug 24, 2026
27 checks passed
YellowSnnowmann added a commit to YellowSnnowmann/tinymemory that referenced this pull request Aug 31, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant