Skip to content

fix(backfill): ask the ingest gate before charging the limit - #144

Merged
YellowSnnowmann merged 2 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/6051-backfill-advance-past-filed
Sep 7, 2026
Merged

fix(backfill): ask the ingest gate before charging the limit#144
YellowSnnowmann merged 2 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/6051-backfill-advance-past-filed

Conversation

@YellowSnnowmann

Copy link
Copy Markdown
Contributor

Summary

The connector-tree backfill (#136, openhuman#6012) could never advance past the first limit documents of a store whose newest documents were already filed. scanned was charged before the ingest gate could answer already_ingested, list_documents yields newest first — exactly the documents the post-#6007 sync path had already treed — and break 'targets on the limit then walled off every later namespace, including the legacy skill-{toolkit} ones the backfill exists for. On the reporting profile every click after the first returned ingested=0 already_present=500 more_pending=true, and 960 pre-migration documents stayed invisible.

The walk now asks the ingest gate before it spends: a document the tree already holds is counted under already_present and costs none of the limit; only a document that still needs reading and filing is charged. The probe goes through a new crate-private connector_item_already_treed beside ingest_connector_item_into_tree, both built on one ConnectorItemIdentity derivation, so the walk asks by exactly the key the funnel files under (the #6007 lesson). The dry run probes the same way, so its scanned is the number of documents still waiting — which is what a host confirms against, and scanned: 0 is its "nothing to repair". A filed document is the one kind the loop handles without awaiting anything, and on a large account they come in long runs, so that arm yields to the runtime per document rather than holding its worker thread for the whole run.

Related issue

tinyhumansai/openhuman#6051

API or behavior changes

No wire shape change: BackfillTreesRequest and BackfillTreesOutcome keep their fields. The meaning of scanned narrows from "documents examined" to "documents charged against limit" (read and filed, or on a dry run the ones a real pass would); documents the tree already holds are reported under already_present only. more_pending now means "documents still waiting to be filed", so a bounded pass converges when called again. Field docs on the bus and API types say so. Not breaking: OpenHuman's UI already reads scanned as "up to N documents to file" and scanned == 0 as "nothing to repair", and the module crate's door test is unchanged and green.

Validation

Commands actually run, with their outcome:

  • cargo fmt --all -- --check — clean
  • cargo clippy --all-targets --all-features -- -D warnings — clean
  • cargo build --all-targets --all-features — clean
  • cargo test --all-features — all suites green (core: 727 passed, 0 failed)
  • bash scripts/ci/engine-containment.sh — holds (backfill.rs reaches the gate through crate::engine, never tinycortex::)
  • RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features — clean
  • Module crate lane (--manifest-path crates/tinymemory-module/Cargo.toml): fmt check, clippy --all-targets -- -D warnings, test --lib backfill — clean; the_backfill_door_files_a_stored_connector_document_through_the_port passes

Tests

Written first, red on main, green with the fix (crates/tinymemory-core/src/backfill_tests.rs):

  • already_filed_documents_do_not_charge_the_limit_so_bounded_passes_converge — 3 documents, limit = 2: pass 1 files 2 and says more; pass 2 recognises 2 and files the third; pass 3 charges nothing (scanned 0, already_present 3) and does not ask to be run again.
  • a_later_target_is_reached_once_the_earlier_one_is_fully_filed — two registry targets, limit = 1 twice: the second pass gets past the filled first target and files the second's document (the break 'targets wall).
  • a_dry_run_tells_already_filed_documents_from_waiting_ones — the preview counts waiting documents, names filed ones, writes nothing, and reads scanned 0 once everything is filed.
  • a_stored_document_is_filed_into_the_tree_and_never_twice now also asserts second.scanned == 0.

crates/tinymemory-core/src/engine/sync_tests.rs:

  • the_gate_probe_agrees_with_the_funnel_it_files_throughSome(false) before the ingest, Some(true) after, also when the caller spells the scope halves with different case/whitespace; a sibling item is its own key; a blank half answers None.

  • a_gate_read_failure_is_tolerated_per_document_and_charges_nothing — the tree directory replaced by a file: every document is skipped with a note, nothing is charged, the pass still answers Ok (the #5820 policy; only corruption aborts, and escalate_or_count has its own unit tests for that split).

Deliberately untested: the walk's Ok(None) arm (an item the funnel would refuse for a blank scope). Targets are built from the registry with blank halves already filtered out, so it cannot be reached through the public entry point; it exists so a future change to resolve_targets cannot silently count such an item as work done.

Coverage (cargo llvm-cov -p tinymemory-core --lib, CI's production-source regex): every new line in engine/sync.rs is covered; in backfill.rs the only uncovered new lines are that Ok(None) arm.

Verification on the reporting profile

Run from a scratch binary against a sqlite .backup copy of the reporting profile (1580 connector documents: 620 already treed by the sync path, 960 pre-migration), with a noop embedder. Default limit throughout.

pass scanned ingested already_present more_pending
dry run, before (v1.15.1) 500 0 0 true
dry run, this branch 500 0 620 true
real #1 500 500 620 true
real #2 458 458 1122 false
dry run after 0 0 1580 false

Dry runs take 45–200 ms; the gate probes are primary-key lookups. (On the live profile the legacy skill-gmail target is additionally skipped as ambiguous because the registry carries stale duplicate rows for a second gmail connection — a separate finding, to be filed against openhuman; the table above is with those rows removed from the copy.)

Documentation

Rustdoc only, in the same change: the module doc of backfill.rs (why the walk asks first, and what went wrong before), BackfillReport field docs, BackfillTreesOutcome / BackfillTreesRequest on the bus types, and the Maintenance::backfill_connector_trees doc in tinymemory-api. docs/ has no page for this door.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

`ingest_connector_item_into_tree` built the tree scope, the per-item
source id and the owner inline. A second reader of the tree — the
backfill, which is about to ask the ingest gate before it spends — would
have to spell the same three names, and two call sites owning that rule
is what produced openhuman#6007. `ConnectorItemIdentity` now derives
them once, with the blank-half guard, and the funnel destructures it.

Behavior-neutral: the same strings reach the pipeline, and the existing
funnel tests stay green.

Refs tinyhumansai/openhuman#6051
The connector-tree backfill could never advance past the first `limit`
documents of a store whose newest documents were already filed. It
charged `scanned` before the ingest gate could answer `already_ingested`,
`list_documents` yields newest first — exactly the documents the
post-#6007 sync path had already treed — and `break 'targets` on the
limit then walled off every later namespace, including the legacy
`skill-{toolkit}` ones the backfill exists for. Every click after the
first reported `ingested=0 already_present=500 more_pending=true`.

The walk now probes the gate through `connector_item_already_treed`,
which asks by the funnel's own identity. A document the tree holds is
counted under `already_present` and costs none of the limit; only a
document that still needs reading and filing is charged, and the dry
run counts the same way, so `scanned` is the number of documents still
waiting and `scanned: 0` means nothing to repair. A filed document is
the one kind the loop handles without awaiting, so that arm yields to
the runtime per document. A gate read failure follows the funnel's
policy: tolerated and noted per document, aborting only on corruption.

No wire shape change; `scanned` narrows from "examined" to "charged",
and the field docs on the bus and API types say so.

Refs tinyhumansai/openhuman#6051
@coderabbitai

coderabbitai Bot commented Sep 7, 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: Team

Run ID: 667b39a2-069c-471f-8418-06656785e555


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

@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

@tinysweeper

tinysweeper Bot commented Sep 7, 2026

Copy link
Copy Markdown

How this change flows

0 changed behaviours across 10 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 45 further behaviours left out to keep the diagram readable.

flowchart LR
  n0["with_config"]:::impacted
  n1["memory_fixture"]:::impacted
  n2["from_workspace_dir"]:::impacted
  n3["join"]:::impacted
  n4["...nal_reader_lists_and_reads_a_local_folder"]:::impacted
  n5["source"]:::impacted
  n1 -->|calls| n2
  n1 -->|calls| n3
  n4 -->|calls| n0
  n4 -->|tests| n0
  n4 -->|calls| n2
  n4 -->|tests| n2
  n4 -->|calls| n3
  n4 -->|tests| n3
  n4 -->|calls| n5
  n4 -->|tests| 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
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 added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Sep 7, 2026
@YellowSnnowmann
YellowSnnowmann merged commit f1c234b into tinyhumansai:main Sep 7, 2026
27 checks passed
YellowSnnowmann added a commit to tinyhumansai/openhuman that referenced this pull request Sep 7, 2026
v1.15.2 carries tinyhumansai/tinymemory#144, which makes
`MemoryMaintenance::backfill_connector_trees` ask the ingest gate before it
charges its limit. Against 1.15.1 the walk re-examines the same `limit`
already-filed documents on every call and never reaches the rest, which is
why the Sources page's "Repair older memories" button reports "Filed 0 (500
already there). More remain." forever (#6051). Nothing in the host changes:
the button, the RPC and the wire types are the same, and the numbers they
show become honest once the module behind them moves.

A patch bump: no contract member was added. The release also carries
tinyhumansai/tinymemory#143 (the remote API), which touches nothing the host
pins.

Four pins move together, as they must:

- `vendor/tinymemory` -> the v1.15.2 commit (258d9e5 -> 9143fe1)
- the registry descriptor: version, release_url, and all 11 platform assets
  with the checksums the release published
- `ARTIFACT_CAPABILITIES_PIN`
- `memory_version` / `memory_sha256` in ci-full.yml, ci-lite.yml and
  e2e-reusable.yml (4 sites), which pin the ubuntu-22.04-x86_64 archive
  independently of the registry

Verified rather than assumed: v1.15.2 has #144's merge (f1c234b) as an
ancestor; all 11 checksums were taken from the release's own checksum.toml
with each archive name and its digest rewritten as one unit; `git diff
v1.15.1..v1.15.2 -- crates/tinymemory-api/src/capabilities.rs` is empty, so
the advertised family list is unchanged; both Cargo.lock files resolve
`--locked` against the new checkout; and the two pin tests
(`the_capability_list_matches_the_pinned_release`,
`the_ci_workflows_pin_the_same_module_digest_as_the_registry`) pass.

Refs #6051
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