Hidden means hidden: dot-prefixed names are never vault material - #140
Merged
Conversation
…136) Rules on the asymmetry #136 raised, in favor of answer (1): `is_hidden` now sits *above* the note/resource dispatch in `collect_vault_files`, so a `.scratch.md` is skipped exactly as `.DS_Store` and `.git/` are. A hidden `.md` gets no b2id stamp, no chunks, no embeddings, no graph presence, and appears in no listing, search, or tree — while its bytes stay untouched on disk (W4). The write side follows, or the ruling would leave a hole: `b2 add .scratch.md` would create a note the walk then never sees — a silent fs/index desync, which is the reason `normalize_rel_dir` already refused dot-segments. That check moves down onto `normalize_rel`, so every authoring destination (note, resource, folder) refuses a hidden path through the one validator they share. Migration, per the issue: a vault that already indexed a dot-`.md` ghost-prunes those rows on the next reindex (#31) and re-dangles inbound links at them (G5 surfaces them). The stamped `b2id:` line stays in the file, so renaming back into the managed subtree re-adopts the same identity — covered by a test. data-model.md §1 gains "Hidden means hidden" as the normative statement, and S2's projected *domain* is stated as the managed subtree; the rule was previously documented only in a walk comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014g8UNXFEZN2KULNxtM2dAh
…137) `resolve_hits` took the top `limit` of the fused ranking and *then* dropped the entries whose chunk no longer resolved, so a dead chunk in a top-`limit` slot cost a live, lower-ranked candidate its place. The race is the posture C1 promises, not a theoretical one: `b2 search` during `b2 reindex &`, or the desktop searching while the fs-watch pulse re-projects, can see a chunk id whose row was replaced between the FTS/vector scan and resolution. Now one pass over the whole ranking, stopping at `limit`. Steady state is byte-identical — every top-`limit` chunk resolves. The issue's scope note asked the same of the two loops above it, so they get it: `search_chunks` retrieved a pool of exactly `limit` while dropping hits whose path/detail lookup missed, and `search` emitted `path: ""` for a miss rather than dropping it — the half-resolved hit its sibling documents refusing. Both now walk a `hit_pool(limit)`-wide pool and stop at `limit`, so a drop is backfilled from the next candidate. `hit_pool` is one function with the reason written down once, replacing `search`'s open-coded 3×. The limit==0 case is *not* the no-op the issue expected under this shape: a `hits.len() == limit` test after the push can never fire at zero, so the loop would have run the ranking to exhaustion and returned everything. Both loops (and `graph_filtered_search`, which already had the after-the-push form) now test before the push, with a test pinning it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014g8UNXFEZN2KULNxtM2dAh
📝 WalkthroughWalkthroughThe change standardizes hidden-path handling across vault walks and authoring validation. It excludes dot-prefixed entries from projection and reindexing. It also updates search resolution to skip missing ranked rows and backfill valid results. ChangesHidden path policy
Search result backfill
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/b2-core/src/ingest.rs`:
- Around line 1241-1243: Update pathspec::is_hidden and the ingest filtering
around it so dot-prefixed non-UTF-8 filenames are recognized as hidden using
encoded-byte inspection rather than rejecting invalid Unicode; preserve existing
behavior for normal paths. Add a Unix-only regression test covering a non-UTF-8
dot-prefixed Markdown filename and verify it is skipped before ingest routing.
In `@crates/b2-core/src/vault.rs`:
- Around line 1019-1024: Ensure zero-limit searches return Ok(Vec::new())
without performing search work: in crates/b2-core/src/vault.rs lines 1019-1024
and 1059-1062, guard after opening the span and before retrieve; in
crates/b2-core/src/search.rs lines 164-167 and 203-207, guard hybrid_search and
graph_filtered_search before embedding, graph loading, or retrieval. Add
coverage in crates/b2-core/tests/search.rs lines 350-369 verifying a zero-limit
façade search succeeds with a mismatched embedder.
In `@docs/design/data-model.md`:
- Line 129: Update the note definition in the data-model design document to
require that the full vault-relative path contains no dot-prefixed segment,
including ancestor directories; retain the existing .md, YAML frontmatter, and
Markdown body requirements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 61b40b4e-070e-4bfb-8eb1-2f634c4819ea
📒 Files selected for processing (10)
crates/b2-core/src/dirs.rscrates/b2-core/src/error.rscrates/b2-core/src/ingest.rscrates/b2-core/src/pathspec.rscrates/b2-core/src/search.rscrates/b2-core/src/vault.rscrates/b2-core/tests/hidden.rscrates/b2-core/tests/search.rsdocs/design/data-model.mddocs/design/invariants.md
…circuit Three threads from the review, all taken: **A dot-prefixed non-UTF-8 filename was not hidden.** `is_hidden` decoded the name with `to_str()`, which answers `None` for bytes UTF-8 rejects, so `.draft-\xFF.md` fell through to `is_some_and`'s false and routed to the note collector. The consequence is worse than "indexed anyway": the walk stores the *lossy* path, which no longer names a file on disk, so every reindex reported a bogus `file no longer exists` skip for a file that should have been invisible. Asked of the name's bytes now — a leading `.` is ASCII and `OsStr`'s encoding is ASCII-compatible, so the test is exact everywhere. Regression test is Unix-gated, since that is where such a name can exist; it reproduced the skip notice before the fix. **`limit == 0` honored the budget but still did the work.** The before-the-push checks stopped results being collected, but `hybrid_search` had already embedded the query, `graph_filtered_search` had already scanned every vector, and the façade had already been through `retrieve` — so a zero-limit search on a model-swapped vault failed with `ModelMismatch` rather than returning nothing. Entry guards on the three retrieval fns and both façade methods. The mismatch case is the observable proof and is what the new test asserts: that guard exists to stop *wrong results*, and there are none to be wrong about. **data-model.md §1** now defines a note by its whole vault-relative path rather than its filename, since an ancestor segment is what excludes `notes/.templates/daily.md` — the rule the code has always applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014g8UNXFEZN2KULNxtM2dAh
…file CI caught what this container could not: the regression test wrote a `.draft-\xFF.md` fixture, and APFS refuses to create one (EILSEQ, "illegal byte sequence"), so the test could not run on the only platform B2 ships on. It was green here because ext4 takes any bytes but NUL and `/`. The fix under test is `is_hidden` reading the name's bytes rather than decoding them, so the predicate is what the test should exercise — the filesystem round-trip added nothing but a platform dependency. Moved to a `#[cfg(unix)]` unit test in pathspec.rs that builds the `Path` from bytes in memory (`OsStr::from_bytes`), which macOS is perfectly happy to do; only *creating* the file is refused. Verified it still discriminates: it fails against the old `to_str` predicate and passes against the byte-wise one. `read_dir` can still hand us such bytes from a mounted volume, so the predicate must stay total over its input — that is the property now pinned, and hidden.rs carries a pointer to it so the walk-level suite doesn't read as covering it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014g8UNXFEZN2KULNxtM2dAh
This was referenced Aug 3, 2026
Closed
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
2 participants
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.
Implements GH #136: a dot-prefixed name (file or folder) is now uniformly excluded from the vault's managed subtree, regardless of its extension or type. This closes a gap where
.scratch.mdcould theoretically be indexed as a note while.DS_Storewas skipped as a resource.Summary
The vault now applies a single hidden-path predicate (
is_hidden) before routing files to notes or resources, ensuring that dot-prefixed entries are invisible to the entire projection — nob2idstamping, no chunks, no embeddings, no graph presence, and no appearance in listings or search. The files remain untouched on disk (W4 invariant), simply outside the index.Key changes
Unified hidden rule in
pathspec.rs: Moved the dot-prefix check fromnormalize_rel_dir(directories only) into the basenormalize_relvalidator, so every authoring destination (note, resource, folder) refuses hidden paths with a consistent error message.Early skip in
ingest.rs:collect_vault_filesnow appliesis_hiddenabove the note/resource dispatch, before recursion. A.scratch.mdis skipped exactly as.DS_Storeis — no special case for Markdown.Search robustness (GH core: search can under-fill
limitwhen a ranked chunk no longer resolves — take(limit) runs before resolution #137): Fixed a concurrent-reindex window where a chunk's FTS row could outlive itschunksrow. Bothsearchandsearch_chunksnow skip unresolved chunks without charging them against the result limit, using a 3× hit pool to backfill from lower-ranked candidates. Addedlimit == 0guards to stop loops before pushing, not after.Authoring guards: All three error variants (
AddDestination,MoveDestination,DirDestination) now reject hidden paths, with updated error messages explaining that b2 does not manage dot-prefixed members.Documentation: Updated
data-model.md§1 with a new subsection "Hidden means hidden" explaining the rule, its rationale (filesystem convention), and the three guarantees (walk skips before routing, files untouched, b2 won't author them). Updatedinvariants.mdS2 to clarify the projected domain is the managed subtree.Test coverage: New
hidden.rsintegration test suite covering note invisibility, no stamping of hidden files, dry-run agreement, rename-to-hide pruning and readoption, and authoring refusals across all destination types. Extendedsearch.rswith tests for dead-chunk skipping and zero-limit behavior.Implementation details
file_name().starts_with('.')on each path segment, applied uniformly byis_hidden.limit.saturating_mul(3), providing headroom for both note dedup and concurrent-reindex drops.resolve_hitsandgraph_filtered_searchnow checkshits.len() == limitbefore pushing, ensuringlimit == 0is honored and the pool's headroom is used correctly.normalize_rel_dirnow delegates entirely tonormalize_rel, eliminating the separate dot-folder check.https://claude.ai/code/session_014g8UNXFEZN2KULNxtM2dAh
fixes #136
fixes #137
Summary by CodeRabbit
New Features
Bug Fixes
Documentation