fix(ingest): a cloud-evicted file is not a deletion - #144
Merged
Conversation
Vaults live in cloud-synced folders, and the clients differ in a way this code did not account for. OneDrive and Google Drive keep the filename and hydrate on read, so `is_file()` stays true. macOS/iCloud Drive does not: when it reclaims disk it replaces `Notes.md` with a sibling stub `.Notes.md.icloud`, so the name leaves the directory and `is_file()` is false — which `ingest_paths` read as "deleted". Every triple the note authored was signed-deleted, its task and card nodes tombstoned, its chunks forgotten. The note comes back when the user next opens it and macOS hydrates the file, so nothing is lost for good — but in between, the DAG has recorded a retraction that never happened and the re-insert that follows it. That is false history in the one structure whose entire value is that its history is true. Meanwhile grounded retrieval answers as though the note does not exist. The full walk was never affected: it cannot tell "gone" from "never seen", so it retracts nothing. Skip the retraction when the stub is present. The opposite error — a stale entry that survives until a real deletion event — is much cheaper, so this errs toward keeping. A real deletion still retracts, pinned by the existing test alongside the new one.
ApiliumDevTeam
added a commit
that referenced
this pull request
Jul 30, 2026
* test: assert something in the tests that asserted nothing Nine assertions compared an unsigned counter against zero — `peer_count >= 0`, `uptime_secs >= 0`, `verification_time_ms >= 0`. Every one of them held for every possible value, including the failure each test exists to catch: `test_add_peer` passed whether or not the peer was added. Replace them with the claim the test is named for where there is one (peer counts), an invariant that can actually break where there is not (time in a power profile cannot exceed total uptime; energy is never negative), and nothing at all where the author's own comment says the value is legitimately zero — with a line explaining why no bound is asserted, so it does not get "fixed" back. Also: `3.14` as an arbitrary test float in eight places, which clippy reads as an approximation of PI. These tests are about storing a decimal, not about geometry, so use a number that does not claim to be a constant. And `aingle_zk`'s memory benchmark had not compiled since `prove_knowledge` settled on `(secret, public_point, message)` — it was still passing the last two swapped. Invisible to `cargo test`, which does not build benches. * style: apply cargo clippy --fix across the workspace Machine-applicable suggestions only, applied by the tool and left otherwise untouched: needless borrows, redundant closures, `is_empty` over length comparisons, `io::Error::other`, derivable impls, unused imports. Mechanical and behaviour-preserving by construction, but it is 40 files of churn in the engine, so it is a separate commit: drop it without losing the fixes in the commit before, which are not mechanical. Twenty-four lints remain that clippy declines to auto-fix because they need judgement — clamp (NaN handling differs), `sort_by_key` where the key borrows, `Default::default()` field reassignment, and a public `from_str` that shadows the trait method and cannot be renamed without breaking callers. Left for a later pass rather than rushed. * style: clear the clippy lints that do not change behaviour Twenty-one of the twenty-four remaining lints, split by the kind of decision each one represents rather than by the lint that reports it. Notation (15). Ten `Config::default()` values built by assigning fields afterwards become struct literals with `..Default::default()`. One loop that existed only to index an array becomes an iterator. One `unwrap` guarded by a separate `is_some` becomes `if let`. One doc list item that rustdoc read as a continuation gets its blank line. And one more assertion that asserted nothing: `!contexts.is_empty() || matches.len() >= 0` held for every usize, so the test passed even when the query returned nothing — the failure it exists to catch. Descending sorts (6). `sort_by(|a, b| b.k.cmp(&a.k))` becomes `sort_by_key(|x| Reverse(x.k))`, which is stable in the same way and removes the class of bug where `a` and `b` are quietly transposed. All six keys turned out to be `Copy`, so none needed the borrow-shaped exception this pass was ready to grant. Formatting is applied to the files this branch touches and to no others, so the diff stays readable. Three lints are deliberately NOT here, because they are decisions and not cleanup, and burying them in a commit called "style" is how a semantic change ships unreviewed. Both `clamp` sites change NaN behaviour, and the `from_str` one shapes a public API. Each has its own issue. * ci: enforce the lint bar the project already declares CLAUDE.md defines `make lint` as `cargo clippy --workspace --all-targets -- -D warnings`. CI ran `cargo clippy -p aingle_minimal --features rest -- -W clippy::all`: one member of seventeen, the library target only, and warnings that failed nothing. It reported green throughout. Behind it: a benchmark that had not compiled in months, assertions that compared unsigned counters against zero and so passed no matter what the code did, and a `flatten()` over `io::Lines` that spins forever on a repeated read error. `--all-targets` is what covers benches and integration tests, and is the only reason the dead benchmark surfaced. `Format Check` was separately red on main, so the aggregate `CI Success` gate has been failing for some time; `cargo fmt --all` here makes it pass and the widened clippy keeps it honest. Note the lints were an onion: with `-D warnings` a failing crate stops everything downstream from being linted at all, so the count grew from 24 to 54 as each layer was cleared. It took four passes to reach a clean run — worth knowing before trusting a single measurement. The three lints that are decisions rather than cleanup carry `#[allow(...)]` with the full reasoning at the site: two `clamp` substitutions that change NaN behaviour, and a public `from_str` that shadows the trait method. Removing each attribute is the definition of done for that decision. * ci: run the checks on every pull request, not just those onto main and dev `pull_request: branches: [main, dev]` meant a PR onto any other branch ran no checks at all. A stack of dependent PRs therefore had no CI until each was retargeted — which is after the review, not before it. Found by building such a stack: three of its five PRs showed no checks whatsoever, and nothing about the interface said why. * fix(ingest): rewrite a fallthrough with `?`, as clippy 1.97 asks `clippy::question_mark` is new in 1.97. Nobody touched this code; the toolchain moved under it. Which is the whole argument for pinning: this repository installs `dtolnay/rust-toolchain@stable`, so the gate added in this PR can go red from a compiler release with no commit behind it. Verified by installing 1.97.0 locally — clippy, rustfmt and the tests are all clean under the exact toolchain the runner used, which 1.96 could not have told me. * fix(ai): stop a single NaN from silently disabling two subsystems Both sites carried a clamp-shaped expression that clippy wanted rewritten. Following that advice mechanically would have introduced a bug in one and papered over a worse one in the other, so each is settled here instead. SurpriseGate. `min`/`max` discard NaN and return the other operand; `clamp` propagates it. But the real entry point is upstream: `record_surprise` feeds Welford's algorithm, and one non-finite sample makes the running mean NaN permanently — every later update keeps it, the adaptive threshold inherits it, and because every comparison against NaN is false the gate stops firing with nothing to show for it. Reject non-finite samples at the boundary, and fall back to the CONFIGURED threshold — the non-adaptive baseline this type already has — rather than to an invented constant, if unusable statistics arrive some other way (deserialised state is not covered by the entry guard). TransactionClassifier::confidence. `partial_cmp().unwrap()` panics on a NaN distance, which `l2_distance` produces from a corrupt centroid, so the process died inside a confidence calculation. `total_cmp` is total and also sorts NaN last, so a corrupt centroid can no longer pass itself off as the nearest one. And with every distance unusable the old code fell past the `second_dist > 0.0` test to return 1.0 — FULL confidence at the exact moment there is no information. It now answers 0.5, the same neutral value it already returns when there are no centroids at all. Five tests, each pinning a failure that was silent: a poisoned mean, a NaN threshold, the panic, and full confidence from no information. * refactor(dag): ExportFormat implements FromStr instead of shadowing it `ExportFormat::from_str` was an inherent method returning `Option`, which shadows `std::str::FromStr::from_str` and returns a different shape from it. A caller reading `ExportFormat::from_str(..)` could not tell which one they were getting, and got no reason for the failure either way. Implement the trait with an error that carries the offending input and knows the accepted spellings. The list of valid formats lived in the REST handler, where it could drift away from the parser with nothing to catch it — it already advertised "dot, mermaid, json" while the parser also accepted "graphviz" and "md". Now the type owns it and a test asserts the message names every spelling the parser takes. One caller migrated. `cargo clippy --workspace --all-targets -- -D warnings` and `cargo fmt --all --check` are both clean with no `#[allow]` left anywhere from this series. * chore: pin the toolchain and declare an MSRV that is true Two blockers for tagging a release, both of the same shape as the lint gate: a number nothing verified. PIN. The workflows installed `dtolnay/rust-toolchain@stable`, so on 2026-07-29 the runner moved to clippy 1.97, `question_mark` arrived, and CI went red with no commit behind it. Sixteen installations across four workflows now take `RUST_VERSION`, declared once. Not a `rust-toolchain.toml` alone: that action exports RUSTUP_TOOLCHAIN, which takes precedence over the file, so the pin would be ignored exactly where it is needed. The file is here too, for local parity, and the note says they must move together. MSRV. Twelve crates said 1.83, the CI environment said 1.85, CLAUDE.md said 1.83, and the truth was none of them. Bisected: 1.85 fails (`cranelift` needs 1.90), 1.90 fails (`wasmer` needs 1.91), 1.91 builds. Nothing had ever noticed because the MSRV job ran `cargo check -p aingle_minimal` — the single member with no wasmer in its tree. It now checks the whole workspace, and the figure lives once in `[workspace.package]` with thirteen members inheriting it. Uniform rather than per-crate deliberately: a lower number on a crate never built at that version is the same unverified promise being removed here. Raising the declared MSRV also lets clippy suggest APIs that were too new before, which surfaced eight `manual_is_multiple_of` sites. Applied. Verified under both toolchains that now matter: clippy and rustfmt clean and 2248 tests green on the pinned 1.97.0, and `cargo check --workspace --all-targets` clean on the declared 1.91.0. * chore(release): bump workspace versions for aingle 0.9.0
ApiliumDevTeam
added a commit
that referenced
this pull request
Jul 30, 2026
* test: assert something in the tests that asserted nothing Nine assertions compared an unsigned counter against zero — `peer_count >= 0`, `uptime_secs >= 0`, `verification_time_ms >= 0`. Every one of them held for every possible value, including the failure each test exists to catch: `test_add_peer` passed whether or not the peer was added. Replace them with the claim the test is named for where there is one (peer counts), an invariant that can actually break where there is not (time in a power profile cannot exceed total uptime; energy is never negative), and nothing at all where the author's own comment says the value is legitimately zero — with a line explaining why no bound is asserted, so it does not get "fixed" back. Also: `3.14` as an arbitrary test float in eight places, which clippy reads as an approximation of PI. These tests are about storing a decimal, not about geometry, so use a number that does not claim to be a constant. And `aingle_zk`'s memory benchmark had not compiled since `prove_knowledge` settled on `(secret, public_point, message)` — it was still passing the last two swapped. Invisible to `cargo test`, which does not build benches. * style: apply cargo clippy --fix across the workspace Machine-applicable suggestions only, applied by the tool and left otherwise untouched: needless borrows, redundant closures, `is_empty` over length comparisons, `io::Error::other`, derivable impls, unused imports. Mechanical and behaviour-preserving by construction, but it is 40 files of churn in the engine, so it is a separate commit: drop it without losing the fixes in the commit before, which are not mechanical. Twenty-four lints remain that clippy declines to auto-fix because they need judgement — clamp (NaN handling differs), `sort_by_key` where the key borrows, `Default::default()` field reassignment, and a public `from_str` that shadows the trait method and cannot be renamed without breaking callers. Left for a later pass rather than rushed. * style: clear the clippy lints that do not change behaviour Twenty-one of the twenty-four remaining lints, split by the kind of decision each one represents rather than by the lint that reports it. Notation (15). Ten `Config::default()` values built by assigning fields afterwards become struct literals with `..Default::default()`. One loop that existed only to index an array becomes an iterator. One `unwrap` guarded by a separate `is_some` becomes `if let`. One doc list item that rustdoc read as a continuation gets its blank line. And one more assertion that asserted nothing: `!contexts.is_empty() || matches.len() >= 0` held for every usize, so the test passed even when the query returned nothing — the failure it exists to catch. Descending sorts (6). `sort_by(|a, b| b.k.cmp(&a.k))` becomes `sort_by_key(|x| Reverse(x.k))`, which is stable in the same way and removes the class of bug where `a` and `b` are quietly transposed. All six keys turned out to be `Copy`, so none needed the borrow-shaped exception this pass was ready to grant. Formatting is applied to the files this branch touches and to no others, so the diff stays readable. Three lints are deliberately NOT here, because they are decisions and not cleanup, and burying them in a commit called "style" is how a semantic change ships unreviewed. Both `clamp` sites change NaN behaviour, and the `from_str` one shapes a public API. Each has its own issue. * ci: enforce the lint bar the project already declares CLAUDE.md defines `make lint` as `cargo clippy --workspace --all-targets -- -D warnings`. CI ran `cargo clippy -p aingle_minimal --features rest -- -W clippy::all`: one member of seventeen, the library target only, and warnings that failed nothing. It reported green throughout. Behind it: a benchmark that had not compiled in months, assertions that compared unsigned counters against zero and so passed no matter what the code did, and a `flatten()` over `io::Lines` that spins forever on a repeated read error. `--all-targets` is what covers benches and integration tests, and is the only reason the dead benchmark surfaced. `Format Check` was separately red on main, so the aggregate `CI Success` gate has been failing for some time; `cargo fmt --all` here makes it pass and the widened clippy keeps it honest. Note the lints were an onion: with `-D warnings` a failing crate stops everything downstream from being linted at all, so the count grew from 24 to 54 as each layer was cleared. It took four passes to reach a clean run — worth knowing before trusting a single measurement. The three lints that are decisions rather than cleanup carry `#[allow(...)]` with the full reasoning at the site: two `clamp` substitutions that change NaN behaviour, and a public `from_str` that shadows the trait method. Removing each attribute is the definition of done for that decision. * ci: run the checks on every pull request, not just those onto main and dev `pull_request: branches: [main, dev]` meant a PR onto any other branch ran no checks at all. A stack of dependent PRs therefore had no CI until each was retargeted — which is after the review, not before it. Found by building such a stack: three of its five PRs showed no checks whatsoever, and nothing about the interface said why. * fix(ingest): rewrite a fallthrough with `?`, as clippy 1.97 asks `clippy::question_mark` is new in 1.97. Nobody touched this code; the toolchain moved under it. Which is the whole argument for pinning: this repository installs `dtolnay/rust-toolchain@stable`, so the gate added in this PR can go red from a compiler release with no commit behind it. Verified by installing 1.97.0 locally — clippy, rustfmt and the tests are all clean under the exact toolchain the runner used, which 1.96 could not have told me. * fix(ai): stop a single NaN from silently disabling two subsystems Both sites carried a clamp-shaped expression that clippy wanted rewritten. Following that advice mechanically would have introduced a bug in one and papered over a worse one in the other, so each is settled here instead. SurpriseGate. `min`/`max` discard NaN and return the other operand; `clamp` propagates it. But the real entry point is upstream: `record_surprise` feeds Welford's algorithm, and one non-finite sample makes the running mean NaN permanently — every later update keeps it, the adaptive threshold inherits it, and because every comparison against NaN is false the gate stops firing with nothing to show for it. Reject non-finite samples at the boundary, and fall back to the CONFIGURED threshold — the non-adaptive baseline this type already has — rather than to an invented constant, if unusable statistics arrive some other way (deserialised state is not covered by the entry guard). TransactionClassifier::confidence. `partial_cmp().unwrap()` panics on a NaN distance, which `l2_distance` produces from a corrupt centroid, so the process died inside a confidence calculation. `total_cmp` is total and also sorts NaN last, so a corrupt centroid can no longer pass itself off as the nearest one. And with every distance unusable the old code fell past the `second_dist > 0.0` test to return 1.0 — FULL confidence at the exact moment there is no information. It now answers 0.5, the same neutral value it already returns when there are no centroids at all. Five tests, each pinning a failure that was silent: a poisoned mean, a NaN threshold, the panic, and full confidence from no information. * refactor(dag): ExportFormat implements FromStr instead of shadowing it `ExportFormat::from_str` was an inherent method returning `Option`, which shadows `std::str::FromStr::from_str` and returns a different shape from it. A caller reading `ExportFormat::from_str(..)` could not tell which one they were getting, and got no reason for the failure either way. Implement the trait with an error that carries the offending input and knows the accepted spellings. The list of valid formats lived in the REST handler, where it could drift away from the parser with nothing to catch it — it already advertised "dot, mermaid, json" while the parser also accepted "graphviz" and "md". Now the type owns it and a test asserts the message names every spelling the parser takes. One caller migrated. `cargo clippy --workspace --all-targets -- -D warnings` and `cargo fmt --all --check` are both clean with no `#[allow]` left anywhere from this series. * chore: pin the toolchain and declare an MSRV that is true Two blockers for tagging a release, both of the same shape as the lint gate: a number nothing verified. PIN. The workflows installed `dtolnay/rust-toolchain@stable`, so on 2026-07-29 the runner moved to clippy 1.97, `question_mark` arrived, and CI went red with no commit behind it. Sixteen installations across four workflows now take `RUST_VERSION`, declared once. Not a `rust-toolchain.toml` alone: that action exports RUSTUP_TOOLCHAIN, which takes precedence over the file, so the pin would be ignored exactly where it is needed. The file is here too, for local parity, and the note says they must move together. MSRV. Twelve crates said 1.83, the CI environment said 1.85, CLAUDE.md said 1.83, and the truth was none of them. Bisected: 1.85 fails (`cranelift` needs 1.90), 1.90 fails (`wasmer` needs 1.91), 1.91 builds. Nothing had ever noticed because the MSRV job ran `cargo check -p aingle_minimal` — the single member with no wasmer in its tree. It now checks the whole workspace, and the figure lives once in `[workspace.package]` with thirteen members inheriting it. Uniform rather than per-crate deliberately: a lower number on a crate never built at that version is the same unverified promise being removed here. Raising the declared MSRV also lets clippy suggest APIs that were too new before, which surfaced eight `manual_is_multiple_of` sites. Applied. Verified under both toolchains that now matter: clippy and rustfmt clean and 2248 tests green on the pinned 1.97.0, and `cargo check --workspace --all-targets` clean on the declared 1.91.0. * chore(release): bump workspace versions for aingle 0.9.0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Vaults live in cloud-synced folders, and the clients differ in a way this code did not account for.
is_file()true — no harmNotes.mdwith.Notes.md.icloudis_file()false — read as deletedingest_pathsretracted every triple the note authored, tombstoned its task and card nodes, and forgot its chunks.Why it matters more than it looks
Nothing is lost for good — the note returns when macOS hydrates the file. But in between, the DAG has recorded a retraction that never happened, followed by the re-insert. That is false history in the structure whose entire value is that its history is true. Meanwhile grounded retrieval answers as though the note does not exist.
The full walk was never affected: it cannot tell "gone" from "never seen", so it retracts nothing.
The trade-off, stated
Skipping the retraction when the stub is present errs toward keeping a stale entry over writing a false one. If a note is genuinely deleted while a stub survives, it stays indexed until a real deletion event arrives. That seemed the cheaper error; it is a judgement and it is reversible.
Verification
ingest_paths_retracts_a_deleted_filestill passes: a real deletion still retracts.