Conversation
This was referenced Jul 2, 2026
Closed
Closed
Closed
Wave 4 / Search: re-baseline #358 <30% index-size target + binarize the JSONL field-map sidecar
#380
Closed
This was referenced Jul 10, 2026
…parator merge.rs: the per-layer rank sort now uses score DESC, then FileId ASC as the tiebreaker (AD-409-4). Previously, equal-score entries within a single layer had non-deterministic relative ranks because the sort was only score DESC. With SEED_STRENGTH=2.0 the blast target always ranks first; equal-Jaccard co-change partners now rank deterministically by FileId ASC rather than by arbitrary HashMap/Vec iteration order. Extends the module doc with the per-layer rank determinism invariant. merge_tests.rs: two new AC-3 tests: - test_per_layer_equal_scores_rank_by_file_id_asc: layer [(FileId(7),0.5),(FileId(3),0.5)] weight 1.0 yields FileId(3) first - test_per_layer_jaccard_order_drives_rank_not_file_id: layer [(FileId(0),0.1),(FileId(9),0.9)] weight 1.0 yields FileId(9) first mod.rs: add one additive, truthful clarification to SEARCH_HELP_TEXT (AD-409-8) that the temporal component of --weights now ranks --blast-radius peers by co-change strength. Fixes: #409
…G entry Adds two unit tests to temporal_tests.rs: - cochange_partner_strengths_carries_jaccard_both_directions: verifies cochange_partner_strengths extracts the correct Jaccard for each partner regardless of which column (file_a/file_b) the target appears in (AC-2). - paths_to_file_ids_drops_unindexed_partners_and_excludes_seed_from_count: verifies unindexed partner drops and that the seed is excluded from the reported partner count (AC-7 / AD-409-7). Adds four unit tests to query_tests.rs: - test_ad409_temporal_weight_only_ranks_by_jaccard_not_alphabetical (AC-1) - test_ad409_seed_file_ranks_first_in_temporal_layer (AC-2) - test_ad409_both_layers_accumulate_with_temporal_rank_from_jaccard (AC-5) - test_ad409_non_finite_jaccard_does_not_panic_and_stays_finite (AC-15/NaN) Adds CHANGELOG entry for #409 under ### Changed, documenting the user- visible consequence: blast-radius composite queries now rank co-change partners by Jaccard DESC; --limit 1 returns the seed file.
…-1,4,6,7,16) Adds crates/rskim/tests/cli_search_blast_weights.rs with six acceptance tests against the FX-LINEAR fixture (5 commits; J(anchor,zstrong)=0.60 > J(anchor,aweak)=0.40 > MIN_COCHANGE_JACCARD; byte-wise path order inverts Jaccard order to expose the pre-#409 alphabetical defect): - ac409_1_temporal_weight_only_follows_jaccard (AC-1): --weights 0,0,1 ranks zstrong before aweak; seed ranks first. - ac409_2_composite_temporal_order_equals_standalone_blast_order (AC-6): self-validating invariant — composite --weights 0,0,1 partner order equals standalone --blast-radius order (ADR-007 dog-food proxy). - ac409_3_repeated_identical_query_is_byte_identical (AC-4): two consecutive --json queries produce byte-identical stdout; HashMap iteration order must not reach output. - ac409_4_unindexed_partner_omission_is_disclosed (AC-7 / AD-409-7): partners absent from the lexical manifest produce a stderr notice and are omitted from results without panicking. - ac409_16a_shallow_clone_ties_are_deterministic (AC-16a): two runs on a single-commit shallow clone produce byte-identical path order. - ac409_16b_real_repo_shallow_degrades_gracefully (AC-16b): --blast-radius on the real repo (shallow CI clone) exits 0 with a "temporal" degraded element when temporal ranking is unavailable. fmt and clippy (1.98.0) both EXIT=0. AC-23 / PF-019 satisfied.
…_file_ids The `paths_to_scored_file_ids` function (used by the composite blast-radius query path `run_blast_radius_composite_query` -- weights 0,0,1) was missing the AD-409-7 partial-drop notice that already existed in `paths_to_file_ids`. When some co-change partner paths are absent from the indexed manifest (e.g. files deleted from disk but still recorded in temporal.db via git history), the notice: "skim search: blast-radius: N of M co-change partners not found in the indexed manifest (excluded from scoring)" was never emitted for the composite query path, only for the standalone AST path. Fix: mirror the identical guard from `paths_to_file_ids` in `paths_to_scored_file_ids` (dropped = allowlist_len - scored_len; partner_count = allowlist_len - 1 to exclude the seed). Also fixes the test fixture in `ac409_4_unindexed_partner_omission_is_disclosed`: - Redesigned commit layout so only aweak.rs is anchor.rs's co-change partner (J=1.0), not zstrong.rs (J=0.0 -- never co-changed with anchor in the fixture). The old 3-commit layout gave both aweak and zstrong J=2/3, so the notice would say "1 of 2" not "1 of 1" as the test expected. - Added backup/restore of temporal.db: the build-time ghost filter (AD-408-1) removes co-change rows for files absent on disk, so a naive delete-then-rebuild removes the (anchor, aweak) row from temporal before the query can observe it. Backing up temporal.db before the second build and restoring it afterward creates the genuine "in temporal but not in lexical manifest" state the notice path is designed for. - Added `find_temporal_db` helper to locate temporal.db in the per-root cache subdir without replicating the SHA256 path logic in tests. Closes: AC-7 / AD-409-7 Gate-1 failure.
… comment
- Extract the duplicated eprintln! drop-notice block from
paths_to_file_ids and paths_to_scored_file_ids into a shared private
helper emit_partial_drop_notice(allowlist_len, found). The message
text is now in one place, eliminating the risk of the two call sites
drifting apart. Both call sites retain their AD-409-7 anchor in
their containing doc-comment.
- Rephrase the AD-409-6 comment in run_blast_radius_composite_query
from tombstone language ("sort_unstable_by_key is deleted") to a
forward-looking statement ("rank derivation is delegated to
merge_layer_scores"). The anchor ID is preserved; the phrase
describing a removed artifact is not.
9-pillar self-review of #409 (blast-radius Jaccard ranking). P0-Functionality — AC-7 "exactly one stderr line" was violated on the composite blast-radius arm. execute_query_with_manifest hoisted paths_to_file_ids above the compound/composite dispatch, but that helper has a stderr side effect (the AD-409-7 partial-drop notice) and the composite arm then emitted the same notice a second time from paths_to_scored_file_ids. The FileId allowlist is only consumed by the compound (text+--ast) arm, so it is now built inside that branch — one notice per query, and one fewer O(manifest) pass on the composite arm. The E2E guard now asserts the notice appears EXACTLY once (it previously only asserted `contains`, so the duplicate passed). P0-Functionality — restored the pre-#409 membership semantics for a non-empty blast-radius allowlist that resolves to zero FileIds (every partner and the target deleted from disk, or a target that is not indexed). #409 retyped the AD-413-16 early-out from the resolved HashSet<FileId> to the source path map, which narrowed it to the AnchorDiffers sentinel only; the resolved-empty case fell through and returned the plain lexical hit list under a --blast-radius flag that had contributed nothing — a confident ranking that is not a blast radius (ADR-009). Re-asserted as a companion temporal_layer.is_empty() guard, which is the one that actually mirrors run_compound_query's filter_set.is_empty() early-out as the comment claims. New discriminating regression test; the existing empty-allowlist guard (AC-24) still passes unchanged. P1-Functionality — paths_to_scored_file_ids emitted the partial-drop notice unconditionally, so a fully-unresolved allowlist rendered the nonsense "N of N-1 co-change partners not found". It now mirrors paths_to_file_ids' two-branch guard via a shared emit_no_indexed_files_notice helper, so both arms disclose the same condition with the same wording. P1-Tests — ac409_3_repeated_identical_query_is_byte_identical compared raw stdout bytes including the wall-clock duration_ms field, so AC-4 was one loaded-machine timing jitter away from flaking. It now excludes only that line (and asserts the field is still present so the normalizer cannot silently match everything), and adds a non-vacuous guard (PF-007). Verified: cargo fmt --check, cargo check -p rskim --all-targets, cargo nextest -p rskim (64 blast/ac409/ad409/cochange tests, all 10 AC-24 guards green), rustup run 1.98.0 cargo clippy -p rskim-search and -p rskim --all-features --all-targets -D warnings (both clean). No new unwrap/expect/panic!/direct indexing in rskim-search (AC-23); AD-409-1..8 anchors intact, no orphans (AC-21).
…ac409_16b (PF-007) Two Gate-2 Evaluate alignment fixes: AC-7 / AD-409-7: The old partial-drop count `allowlist_len − found` was off-by-one when the seed itself was absent from the manifest: the seed would be double-counted as a "dropped partner". Fix tracks `seed_resolved` during the manifest scan (the seed carries `SEED_STRENGTH = 2.0`, a value no Jaccard score can equal) and computes: partner_count = allowlist_len − 1 partners_found = found − seed_resolved_as_int dropped = partner_count − partners_found `emit_seed_unindexed_notice()` fires when the seed is absent; the partner-drop notice is suppressed when dropped == 0 even if the seed is missing. Same logic applied to both `paths_to_file_ids` and its scored twin `paths_to_scored_file_ids`. AC-7 regression test `ac409_7_seed_unindexed_notice`: the prior fixture deleted the seed from disk, but `normalize_blast_radius_path` requires the seed to exist on disk, and the working-tree staleness scan (AD-379-5) would also detect the restored file as "1 added" and auto-rebuild. Fix: use a binary (non-UTF-8) seed file — `target.bin` goes to `skipped_entries` during indexing (NonUtf8 content skip), so it is never in manifest entries, exists on disk (normalize succeeds), and the staleness scan sees it as a previously-known skipped file (not newly added). PF-007 / ac409_16b: the byte-identical comparison was vacuous against `duration_ms` timing noise. Fix: parse both JSON outputs and compare only the ranked result-path list, which is the actual determinism invariant.
…xity)
Five confirmed findings fixed:
1. [low/security] Clamp DB-sourced Jaccard to [0.0, 1.0] at trust boundary in
cochange_partner_strengths so no corrupt co-change row (including one with
jaccard == 2.0) can impersonate the seed sentinel. NON_FINITE_JACCARD_FLOOR
moved to module scope; cochange_partner_strengths uses the full finite-range
guard (j.is_finite() && j >= 0.0 && j <= 1.0) instead of is_finite() alone.
2. [medium/architecture] + 3. [medium/complexity] Eliminate the duplicate
resolver: paths_to_file_ids now delegates to paths_to_scored_file_ids
(one .into_iter().map(|(id,_)| id).collect() line) instead of maintaining a
parallel 30-line body. Membership parity, notice dispatch (emit_no_indexed_files_notice,
emit_seed_unindexed_notice, emit_partial_drop_notice), and PF-004 widening
now have exactly one implementation (AD-409-7).
4. [low/architecture] BlastRadiusStrengths alias used in enum variants:
BlastRadiusResolution::Allowed and Filtered.allow now spell the type as
BlastRadiusStrengths (not bare HashMap<String, f64>). The alias is added
to the existing `use super::types::{...}` import so consumers and producers
use the same spelling.
5. [medium/complexity] emit_partial_drop_notice simplified to two parameters
(partner_count, partners_found): partners_found is a direct counter
incremented in the scan loop (not derived from totals after the fact),
removing the seed_bit / saturating_sub juggling and the 15-line proof-by-
comment. The remaining arithmetic is one saturating_sub.
Verified: cargo check -p rskim --all-targets → EXIT=0; cargo fmt exits 0.
…ch, collapse duplicate guards - Extract blast_temporal_layer helper (hoisted before Step 1 / lexical search) handles BOTH the Some(empty) AnchorDiffers sentinel and the "allowlist non-empty but nothing resolves" case in a single early-out, restoring the pre-#409 ordering that avoided a wasted BM25F corpus pass when the temporal layer is unresolvable (finding: medium/performance) - Remove the two split AD-413-16 guards (~90 lines apart) and their duplicate nine-field QueryOutput literals; replace with the single `let Some(temporal_layer) = blast_temporal_layer(...) else { ... }` site before the lexical search (finding: low/architecture duplicate guards) - blast_temporal_layer adds an `allowed.is_empty()` short-circuit for AnchorDiffers: skips paths_to_scored_file_ids to avoid a redundant "0 indexed files" stderr notice on top of the upstream mismatch notice - Renumber steps: old Step 3/4/5/6 become Step 2/3/4/5 to reflect the temporal layer now being resolved before Step 1 - Remove stale inline comments: "When blast_radius_paths is None (temporal DB absent or not requested), degrades to lexical-only ranking via the empty default" — wrong on both counts (finding: medium/complexity) - Update blast_temporal_layer docstring to accurately describe when paths_to_scored_file_ids is and is not called (finding: low/architecture stale contract comment) cargo check -p rskim --all-targets: EXIT=0, zero warnings
…on+reliability) Five confirmed findings fixed: 1. [medium/consistency] Normalize three blast-radius stderr notice prefixes to the established `"skim search: note: "` convention (all three previously used `"skim search: blast-radius: "` or `"skim search: blast-radius filter"`). Export two as `pub(super) const` — BLAST_RADIUS_SEED_UNINDEXED_NOTICE and BLAST_RADIUS_PARTNER_NOT_FOUND — so intra-crate tests can assert against a single source of truth (consistent with WEIGHTS_FULLY_INERT_NOTICE and WEIGHTS_TEMPORAL_INERT_NOTICE in query.rs). 2. [medium/consistency] Fix stale rustdoc on `paths_to_file_ids` that claimed it was "the single source of truth for all three blast-radius call sites" — now correctly describes it as the membership-only twin of `paths_to_scored_file_ids` with its two actual call sites (compound text+AST arm and `resolve_blast_radius_file_ids`). 3. [low/consistency] BlastRadiusStrengths type alias already applied consistently in commit 442fa92 (confirmed in-place, no additional change needed). 4. [low/regression] Replace per-entry float comparison for seed identity (`jaccard == SEED_STRENGTH` in the scan loop) with a single pre-computed `seed_path: Option<&str>` extracted from the allowlist by `find_map`. Seed identity is now carried explicitly as a path rather than inferred from a sentinel value. Seedless allowlists (e.g. AC-24 guard tests with all-1.0 maps) set `seed_path = None`, suppressing the seed-unindexed notice entirely and preventing a false user-facing disclosure. Also correct `partner_count` computation: `allowed_paths.len()` when no seed is present (not `len() - 1`). 5. [low/reliability] `paths_to_file_ids` now uses explicit `HashSet::with_capacity(scored.len())` to match the pre-sized pattern established by `paths_to_scored_file_ids`, making the allocation intent clear. Verified: cargo check -p rskim --all-targets → EXIT=0; cargo fmt exits 0.
…_assert, fix pre-existing compile/clippy issues query.rs: - Extract `fn empty_output(config, ctx, vm_label)` helper and replace the three byte-identical QueryOutput zero-result literals in run_compound_query and run_blast_radius_composite_query. Adding a field to QueryOutput now requires one edit instead of three (finding 1: low/complexity). - Add `debug_assert!(config.blast_radius_paths.is_some(), …)` at the top of run_blast_radius_composite_query to make the pre-condition checkable rather than narrative (finding 4: medium/reliability, reliability.md). - Document the stderr disclosure asymmetry between the AST and composite blast- radius arms for the AnchorDiffers sentinel (finding 3: low/regression). The composite arm correctly skips the "0 indexed files" notice; the AST arm emits it (intentional, pre-#409 parity); both sites now explain why. - Findings 2, 4 (stale comment), and 5 were already fixed by the prior commit (282bf03) which removed the stale "degrades to lexical-only" sentence and rewrote the temporal-layer guard into blast_temporal_layer. The debug_assert above completes finding 4 per its "either/or" fix options. temporal.rs (pre-existing uncommitted work): - Export BLAST_RADIUS_SEED_UNINDEXED_NOTICE and BLAST_RADIUS_PARTNER_NOT_FOUND constants so tests can assert against a single source of truth. - Add "note: " prefix to all blast-radius stderr notices for consistent style. - Identify the seed by path (seed_path: Option<&str>) not by float value so seedless allowlists never trigger a false seed-unindexed notice. - Fix clippy::manual_range_contains in cochange_partner_strengths (j >= 0.0 && j <= 1.0 → (0.0..=1.0).contains(&j)). - Refactor paths_to_file_ids to use HashSet::with_capacity. tests (pre-existing uncommitted + new fixes): - common/git_fixture.rs: new shared helpers (git_init, git_commit, write_and_stage, now_epoch) extracted from per-test duplicates. - common/mod.rs: add `pub mod git_fixture` declaration so the module is reachable via `use common::git_fixture::{…}`. - cli_search_blast_weights.rs: switch from inline git helpers to common::git_fixture (pre-existing change; was missing the mod.rs declaration that caused cargo check failures). - cli_temporal_first_parent.rs: remove unused `use std::fs` import (clippy -D warnings at 1.98 toolchain). Closes #409
…(PF-007, AD-409-7, AC-12/13/19)
Finding 1 (ac409_16b): replace run_search_raw → run_search_raw_ok, unwrap_or(Null)
→ .expect(...), remove optional if-let degraded branch — assertion is now
unconditional because make_merge_commit_fixture() guarantees temporal is always
unavailable (merge-commit HEAD + shallow clone = zero non-merge commits).
Finding 2 (ac409_4, ac409_7): replace run_search_raw → run_search_raw_ok (AC-7
exit-0 requirement now verified), add positive JSON-parse + path-in-results
assertions so the tests are no longer vacuously green on stdout (PF-007).
Finding 3 (ac409_7b): new test — seed absent AND ≥1 partner dropped combination.
Both AD-409-7 notices must fire ("target file not found" + "1 of 2 co-change
partners not found"). partner_count excludes the seed slot (saturating_sub(1)),
so "1 of 2" is the correct count, not "2 of 2" or "1 of 3".
Finding 4 (ac409_ac12, ac409_ac13, ac409_ac19): new tests for AC-12 (--weights
0,0,0 returns zero results), AC-13 (per-page pagination is disjoint and
concatenates to the full unpaginated result), and AC-19 (text + --blast-radius
+ --hot exits 0 with parseable JSON).
Finding 5 (extract): common/git_fixture.rs extracted in a prior commit; this
commit's temporal.rs hunk is doc-comment-only clarification (no behaviour
change) staged from an earlier phase of the #409 review cycle.
…g at all sites Finding 3: query.rs:535 called AC-7 "exactly one stderr line" while temporal.rs accurately states "at most two" lines (seed-unindexed + partner-drop are separate disclosures) and that "exactly one of the two functions runs per query dispatch path." Reconcile by rewriting the query.rs comment to say what AC-7 actually governs — no double-reporting across dispatch paths — matching temporal.rs. Also clarify temporal.rs:433 "exactly once per query" → "each fires at most once per query" to remove the ambiguity with the "at most two" bound. Finding 4: remove before/after transition narration at four locations: - query.rs AD-409-7 comment: drop "hoisting it above the dispatch made ... TWICE" narrative; keep the invariant (one function per dispatch path, no double-report). - query.rs early-out comment: "restoring the pre-#409 early-out ordering" → "matching the early-out ordering of the standalone temporal arm." - temporal.rs BlastRadiusResolution::Allowed: drop "Retyped from HashSet<String> ... instead of uniform 1.0" narration; replace with end-state description of the type's semantics and downstream contract (AD-409-2). Findings 1 and 2 from this batch were already resolved by prior commits (282bf03, commit range on this branch). compile: cargo check -p rskim --all-targets → EXIT=0
- Remove stale #526 from three comment sites (finding 1); #483 is the correct tracker for the --json/degraded key for dropped partners. #526 is a different defect (text+--blast-radius+--hot composite discard). - Expand paths_to_file_ids AD-409-7 paragraph (finding 2) to explicitly document the zero-match branch ("matched 0 indexed files") alongside the seed-unindexed and partial-drop branches, mirroring the wording already used in paths_to_scored_file_ids. - Replace dead name resolve_blast_radius_filter with the live symbol resolve_blast_radius_file_ids at both doc sites (finding 3): resolve_blast_radius_paths() shared-core sentence and resolve_blast_radius_file_ids() call-site bullet.
The sentence in SEARCH_HELP_TEXT describing Jaccard-strength ranking cited (AD-409-8) — an internal decision-record ID that end users cannot resolve. Every other cross-reference in that help text uses a public issue number (#199, #200, #377, etc.). Fix: replace (AD-409-8) with (#409) in the printed string, and place the AD-409-8 anchor in the adjacent /// doc comment above SEARCH_HELP_TEXT, consistent with how all other AD-409-* anchors are written (rustdoc / // comments, never user-facing output). AC-21 grep is still satisfied. Also applies cargo fmt cleanup to ast_tests.rs (std::collections::HashMap qualified paths collapsed to local use + bare type).
…up, ordering assertion
Fixes four confirmed findings from the review batch:
[medium/testing BLOCKING] Finding 4 — fix fixture byte-order confusion.
- Rename create_ac409_project seed file anchor.rs → zzanchor.rs so it
sorts LAST (aweak < zstrong < zzanchor). The pre-#409 defect placed the
alphabetically-first file at rank 1; with zzanchor at FileId(2) the
seed-ranks-first assertion can only pass via SEED_STRENGTH, never by
accident from FileId-ASC ordering.
- Correct four stale byte-order comments in query_tests.rs (create_ac409_project
doc, test_ac409_temporal_weight_only doc) and cli_search_blast_weights.rs
(module doc lines 16-18, ac409_1 doc, inline comment): anchor < aweak <
zstrong ('n' < 'w'), not aweak < anchor < zstrong as previously stated.
[low/consistency] Finding 3 — rename test_ad409_* → test_ac409_* (4 tests)
to match surrounding AC-criterion naming (test_ac12_*, test_ac13_*, helper
create_ac409_project, E2E tests ac409_1_*, ac409_2_*, …).
[low/complexity BLOCKING] Finding 1 — replace the self-negating comment and
two redundant is_finite() asserts at the end of test_ac409_both_layers…
with a concrete ordering assertion: hit.rs (fused score 0.5/61+0.2/62)
outscores zpartner.rs (0.2/61) and must appear first in results.
[low/complexity SHOULD-FIX] Finding 2 — extract duplicated fixture code:
- Add temporal_only_weights() → CompositeWeights (replaces 3 identical
8-line literals in query_tests.rs).
- Add blast_config(root, cache, text, allowed, weights) → QueryConfig
(replaces 3 identical 12-field QueryConfig blocks).
- Add write_and_stage_bytes(dir, filename, &[u8]) to common/git_fixture.rs
(bytes sibling of write_and_stage); update import and use it in
ac409_7_seed_unindexed_notice, eliminating two inline git-add blocks.
Refs #409
…ank claim, add target-file notice Finding 1 (medium): The 'returns the seed' sentence under User-visible consequences was unconditionally phrased, implying the seed always wins at --limit 1. Added qualifier 'with a temporal-dominant --weights (e.g. 0,0,1)' and a follow-up clause explaining that at default weights (0.5,0.3,0.2) a file with a strong lexical match may still outrank the seed in the composite score — matching the plan's own AC-5 worked example. Finding 2 (low): The unindexed-partners stderr disclosure bullet omitted the second notice: when the blast-radius target file itself is absent from the indexed manifest, emit_seed_unindexed_notice fires a separate line on stderr. Added a clause describing that case alongside the existing partner-count notice.
…nest comment The paths_to_file_ids_drops_unindexed_partners_and_excludes_seed_from_count test asserted locally-recomputed arithmetic that always held given the preceding cardinality assertions: dropped = allowed.len() - file_ids.len() -- the OLD formula partner_count = allowed.len() - 1 -- constant from setup Both assertions were tautologies: 'dropped == 1' follows automatically from 'allowed.len() == 3' and 'file_ids.len() == 2', and the OLD formula (all_dropped / all_total) is the one emit_partial_drop_notice REPLACED with (partner_count, partners_found) in production. Sub-case B had no stderr capture, so a regression that fired the notice on zero drops would not be caught. Fix: delete the tautological blocks from both sub-cases. The cardinality and FileId membership assertions (file_ids.len(), contains/not-contains) are the real coverage. The exact notice text and the production formula are covered end-to-end by ac409_4_unindexed_partner_omission_is_disclosed. Update the docstring to state this split explicitly.
…-008) Replace stale pre-#409 type and symbol names in both feature-knowledge files: cmd-search/KNOWLEDGE.md: - Frontmatter keywords: replace cochange_partner_paths with cochange_partner_strengths; add paths_to_scored_file_ids, BlastRadiusStrengths, BlastRadiusResolution, SEED_STRENGTH - Architecture map: same symbol updates in temporal.rs line - resolve_blast_radius_paths signature: Option<HashSet<String>> → BlastRadiusResolution - QueryConfig struct: blast_radius_paths: Option<HashSet<String>> → Option<BlastRadiusStrengths> - Symbol index for temporal.rs: add new symbols, remove cochange_partner_paths - Anti-pattern: mention paths_to_scored_file_ids alongside paths_to_file_ids search-temporal/KNOWLEDGE.md: - Dispatch flow: HashSet → BlastRadiusStrengths + BlastRadiusResolution - Blast-radius combined-mode section: update prose, code block, and key-insight paragraph - Anti-pattern section: blast_radius_paths HashSet → BlastRadiusStrengths - types.rs key-files entry: add BlastRadiusStrengths type annotation
…d helper - `paths_to_file_ids` (temporal.rs): replace the manual `with_capacity` + `extend` with an iterator chain `.collect()`. `Vec::into_iter()` carries an exact `size_hint`, so `HashSet::from_iter` pre-sizes identically; the three-line manual form added no allocation benefit and its comment was inaccurate. Idiomatic collect is shorter and easier to audit. - `cli_search_blast_weights.rs`: extract the eight-line `StdCommand` body shared by `run_search_raw` and `run_search_raw_ok` into a private `search_command` helper. Both callers delegate to it; the exit-check logic in `run_search_raw_ok` is unchanged. Future env-var or flag changes need one edit instead of two.
Three documentation-accuracy defects in the #409 diff (P2-Documentation, ADR-001 "fix noticed issues immediately"; PF-008 doc-drift). 1. search-temporal/KNOWLEDGE.md claimed the text + --blast-radius path "converts to a FileId allowlist (HashSet<FileId>) and injects as SearchQuery.file_filter". That is false for the composite arm: run_blast_radius_composite_query deliberately runs the lexical search with NO file_filter over a wide K x limit pool (UNION semantics, truncate-LAST). Only the compound text+--ast arm sets a file_filter. #409 rewrote these sentences and strengthened the false claim. Replaced with a two-arm table and an explicit anti-pattern forbidding a file_filter on the composite arm. 2. The same file's new resolution snippet said "::Degraded/Filtered -> blast_radius_paths stays None". Filtered maps to Some(EMPTY), not None -- that Some(empty) != None distinction is AD-413-16 / AC-9 and is load-bearing (collapsing it falls through to an unfiltered lexical search on a wrong-repo temporal DB). Corrected the four-arm mapping and called the invariant out. 3. cmd-search/KNOWLEDGE.md said paths_to_file_ids / paths_to_scored_file_ids "use binary search" (O(n log n)). Neither does -- there is no binary_search in temporal.rs; each performs ONE linear pass over sorted_paths with an O(1) map lookup per entry (AD-409-5). #409 extended the fabricated rationale from one function to two. Corrected the claim and restated the real requirement: FileId is the slice POSITION, so the slice must be manifest.sorted_paths() or the FileIds are silently wrong. Also replaced a line-number reference in the run_blast_radius_composite_query precondition comment ("execute_query_with_manifest line ~587") with the symbol name of the dispatch gate -- it was already stale by 9 lines in its own commit. Comment/markdown only; no behaviour change. Verified: cargo fmt --check, cargo check -p rskim --all-targets, rustup run 1.98.0 cargo clippy -p rskim-search/-p rskim --all-features --all-targets -- -D warnings (all exit 0).
…Jaccard), seed first # Conflicts: # crates/rskim/tests/cli_temporal_first_parent.rs
…he measured structural amplification (ADR-003)
…ed top window; document the zero-result early-out Replace the near-vacuous ac409_ac19_blast_hot_exits_zero (which only asserted exit 0 and JSON parse) with ac409_ac19_blast_hot_resorts_fused_ top_window_by_hotspot — a real AC-19 guard. New FX-HOT-DIVERGE fixture makes the fused (Jaccard-based) ordering and the hotspot ordering visibly different: zzrare.rs is the strong Jaccard partner (J=0.75) with only old commits (low hotspot), while active.rs is the weak partner (J~=0.286) with many recent solo commits (high hotspot). Fused temporal order: [seed, zzrare, active]. After --hot re-sort of the top resort_window(3)=100 entries: [active, seed, zzrare]. The test computes the expected order in-test from two independent CLI queries (fused without --hot, standalone --hot for scores) and cross-references against the actual compound result. A non-vacuity guard asserts the two orderings differ before comparing. resort_window arithmetic is replicated in-test with a comment naming the source function. CHANGELOG: document the temporal_layer.is_empty() early-out (AD-413-16 semantics restored) — a blast-radius allowlist whose partners all resolve to zero indexed files now returns zero results with the partial-drop notice, rather than silently falling back to an unfiltered lexical list.
…number
F-C4-01 (dog-food 2026-09, corpus c4-merge, probe A-02).
Root cause (the reported hypothesis was falsified): the minified-line guard
plays no part. A 955-byte line whose marker is a normal word token is scored
and anchored correctly. The failing shape is a marker GLUED to a long run
(`AAAA...AAAAmk4_longline_marker` is one 919-byte word token), which the
AD-411-7 token_length gate correctly refuses to count as a whole-token match.
search_exact_intersection then emits the file as a substring-only candidate,
score 0.0 with an EMPTY match_positions vec, so it can stay in the result set
for git-grep recall parity (ADR-007).
The defect is in snippet.rs::extract_snippet_and_verify: the AD-396-5 guard
nulls the content-derived anchor whenever match_positions is empty, which it
intended as the "<3-byte short query" signal. Substring-only candidates share
that shape, so a verified match came back with line_number/line_range/snippet
all null even though substring_first_anchor had just located the occurrence.
Narrow the guard to its real condition, a query that produces no trigram
(extract_query_ngrams(...).is_empty() -- the same predicate the reader uses to
route to short_query_fallback), documented as AD-396-8. Short queries stay
snippet-less; no line-length cap is introduced, so a 20 KB single line anchors
too. Score 0.0 for substring-only candidates is unchanged and deliberate.
Tests:
snippet_tests.rs::test_substring_only_long_token_gets_anchor_ad396_8
snippet_tests.rs::test_substring_only_20kb_line_gets_anchor_ad396_8
tests/cli_search_anchor.rs (new E2E through the binary):
f_c4_01_long_line_substring_match_is_anchored
f_c4_01_20kb_line_substring_match_is_anchored
f_c4_01_short_query_remains_unanchored (AD-355-7 scope guard)
…ast-radius
F-C2-02 (dog-food 2026-09, corpus c2-shallow, probe 409-09).
Root cause: run_temporal_standalone passes `sort` to open_temporal_state_for,
whose emptiness probe is per-sort-dimension. With `--blast-radius FILE` as the
only temporal flag there is no sort, so `None` is passed and no probe runs (G-3
keeps co-change emptiness from borrowing the empty/shallow wording). An
entirely empty temporal.db therefore reached query_standalone and produced
`{"mode":"blast-radius","target":...,"total":0,"results":[]}` with no degraded
key and an empty stderr, byte-indistinguishable from a healthy DB in which the
file simply has no co-change partners. The composite arm
(resolve_blast_radius_paths) has always disclosed this case, so the two arms
disagreed about the same DB state. Every other reason (missing / corrupt /
newer-schema / repository-mismatch / not-a-repo) was already disclosed by the
Unavailable arm.
Fix (AD-414-24): after the DB opens, the standalone arm runs the composite arm's
exact predicate — partner set empty AND hotspot table empty — and reports the
same element (requested "blast-radius", applied "none", reason from the SSOT
DegradedReason) plus the stderr notice. Mirroring the composite predicate keeps
a synthetic DB with cochange rows but no hotspot rows from being misreported as
empty; the cheap dimension probe is evaluated first so a healthy DB never pays
for the partner lookup. The stderr/JSON emit shape is extracted into
report_standalone_degraded so both standalone early-outs stay identical, and
empty_temporal_state becomes the single query-time producer of Empty.
Tests (cli_search_degraded.rs):
f_c2_02_standalone_blast_radius_discloses_degraded_state (empty + newer-schema)
f_c2_02_healthy_zero_partner_blast_radius_stays_quiet (negative)
F-C2-01 (dog-food 2026-09, corpus c2-shallow, probe 414-07).
Root cause: DegradedReason::Empty already had a shallow branch in `cause` and
`full_message`, keyed on `detail == "shallow"`, but the only producer of that
detail was the BUILD-time zero-row notice in temporal_build.rs. Every
query-time producer constructed `Empty` with an empty detail, so on a genuine
`--depth 1` clone (meta.is_shallow = 1, confirmed in the DB) the stderr notice
and degraded[].message advised `skim search --rebuild`, which re-derives the
same zero rows because the history is not in the clone. `remediation` was
`&'static str` and detail-independent by design, so degraded[].remediation
could not carry the unshallow advice at all.
Fix (AD-414-25):
- empty_temporal_state now reads META_IS_SHALLOW through the already-open
connection (no extra DB open) and sets the "shallow" detail, so all three
query-time arms inherit the branch from one builder.
- DegradedReason::remediation_for(detail) becomes the single emit-site entry
point; `remediation` is private and holds the detail-independent base table.
- SHALLOW_EMPTY_REMEDIATION is now the one source for both the message tail
and the JSON remediation, so a DegradedJson can never advise something its
own message contradicts (AD-414-1 SSOT).
A meta read failure or an absent row degrades to the non-shallow wording; a
shallow claim is never fabricated.
Tests:
temporal_tests.rs::f_c2_01_empty_notice_branches_on_shallow_detail
temporal_tests.rs::f_c2_01_empty_temporal_state_reads_meta_is_shallow
cli_search_degraded.rs::f_c2_01_shallow_clone_query_notice_names_unshallow
t19b_remediation_text_conformance updated for the new entry point.
…ge partners F-C1-01 (dog-food 2026-09, corpus c1-healthy, Gate-2 #409 AC-20). Root cause: resolve_blast_radius_paths inserted the target with SEED_STRENGTH unconditionally, after the zero-partner branch had already printed "no co-change data for X". With an empty partner set the allowlist was therefore {seed: 2.0}, which paths_to_scored_file_ids turned into a one-element temporal layer, so the composite arm returned the seed as its own co_change_partner scoring exactly temporal_weight / 61 (0.2/61 with default weights) even though no co-change relation exists. Plan check before changing anything: #409 AC-2 pins the seed at temporal rank 1 "via a finite sentinel strictly greater than the Jaccard maximum" and its fixture (anchor.rs + zstrong.rs + aweak.rs) always has partners; AC-20 requires a target with no co-change rows to "exit 0, emit the existing degraded/no-co-change stderr notice unchanged, and MUST NOT fabricate any ranking". No criterion asserts the seed appears with zero partners, so there is no conflict: the fabricated single-seed ranking is precisely what AC-20 forbids. Fix (AD-409-9): inject the seed only when at least one cochange row resolved. The resulting empty allowlist is the "contributes nothing" sentinel that blast_temporal_layer already early-outs on (ADR-009), so all blast-radius arms return zero results — matching the standalone arm for the same target. paths_to_scored_file_ids now returns before its manifest scan on an empty allowlist, so the redundant "matched 0 indexed files (allowed 0 paths, ...)" line no longer follows the notice that already explained the situation. Behaviour change recorded in CHANGELOG: the composite arm no longer surfaces text matches inside a zero-partner target. Test: cli_search_blast_weights.rs::ac409_9_zero_partner_seed_is_not_ranked (asserts results == [] with the notice intact, AND that a target WITH a partner still ranks first, so the change cannot over-reach).
414-12-message (dog-food 2026-09, corpus c3-absent-rows, finding F-C3-01).
The stderr notice is printed as `skim search: {message}` by the CLI output
layer, so the JSON `message` field is the notice text WITHOUT that prefix.
CLAUDE.md, the AD-414-1 rustdoc on `degraded_notice` / `DegradedJson::new` /
`DegradedJson.message` / `blast_radius_degraded_msg`, and the #414 CHANGELOG
entry all read as a byte-identity claim between the JSON field and the stderr
line, which a `message == first_stderr_line` comparison fails.
Resolution is documentary: the code is correct as it stands. The prefix is
terminal presentation, not part of the machine-readable contract, and adding it
to the JSON would make every consumer strip it. The docs now state the actual
relationship (`stderr.contains(message)` holds byte-for-byte; the two strings
are not equal) and tell a consumer that wants the stderr line verbatim to
prepend the prefix.
The CLAUDE.md `search` paragraph also picks up the two contract additions from
this batch: the shallow-clone remediation (AD-414-25) and the standalone
`--blast-radius` degraded array (AD-414-24).
… wave/wave4-search # Conflicts: # .github/workflows/ci.yml # crates/rskim/src/cmd/search/mod.rs
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.
This branch lands the complete three-layer code search system into
main: a lexical trigram index, a temporal co-change index, an AST structural index, and a compound engine that intersects and re-ranks all three. It includes the Wave 4 feature foundation (tickets 4a through 4d), three rounds of dog-fooding correctness work (waves 4/5 correctness tickets #393 through #412), and the 2026-08/09 robustness pass (#413, #414, #407, #409). The branch also removes theskim search indexlegacy positional subcommand (#375), which supersedes the v2.11.0 disambiguation shipped in #385:skim search indexnow runs a lexical query for the word "index". No part of this search surface was previously inmain.Branch tip:
0f0d0bd(2026-09-05)Tickets merged on this branch
Wave 0, Foundation
Wave 1, Lexical Index
Wave 2, Temporal Index
Wave 3, AST Structural Index
skim search --ast <pattern>(Wave 3g)query.rsmodule split and DRY helpersWave 4, Compound Engine
--weights(Wave 4b)--astcomposes with temporal/co-change (Wave 4d)Wave 4 dog-fooding round 1 (2026-06-23)
--astintersection: complete by construction--rebuild/--buildpopulatetemporal.db; bare--hotself-heals.skpostWave 4 dog-fooding round 2 (2026-06-28) and cleanup
--astfile-ID ordering skew fix (manifest format v2 to v3)--aststructural verify gateskim search indexshadows the query term "index" — remove the legacy positional #375 Removeskim search indexlegacy positional subcommand--weightsnow active on pure-lexical and compoundtext+--astpaths--riskyvolume weighting<30%target with grounded ADR-003 ceiling)<root>/.skim/Wave 4/5 dog-fooding round 3 (2026-07 through 2026-08)
--phrase/--neartoken-exact verification at the CLI gate--astreturns results for all five synthetic marker patterns--astline anchors unified and corrected--rootnow exits with a clear error--phrase --near Ncomposition, inert-flag notices--offsetnow honored on all standalone temporal armsast_coveragein JSON and stats--end-of-flags separator2026-08/09 robustness pass
commondir;--install-hooks/--remove-hooksroute to the shared hooks directory;--root <subdirectory>adopts enclosing repository HEAD; reason-specific temporal-arm degradation messages; path-traversal guard on symbolic refs (also closes 🟡 P2: Harden symbolic-ref path validation,ref: refs/../../../outside-shaescapes the git dir and the escaped SHA is persisted (tracking for #413 Step 1) #482)--rebuildis no longer a silent no-op when a build-backoff sentinel is set; corrupttemporal.dbis now discarded and rebuilt;degradedJSON array on unavailable temporal data;temporal_stateandstalenessin--stats --json; shallow-clone cause and remedy in query-time empty notices; standalone--blast-radiusdiscloses empty temporal DB; unborn-HEAD repositories handled correctlyskim heatmappopulation (hot/risky scores previously undercounted approximately 3x on branch-heavy workflows);TEMPORAL_DATA_VERSIONself-heal on upgrade (one slow query per root, then fast)--blast-radiustemporal axis now ranks co-change peers by actual Jaccard strength; seed ranks first via a finite sentinel;merge_layer_scoresper-layer sort is now a total comparator; unindexed co-change partners disclosed on stderrBehaviour changes users will notice
The items below are from the
[Unreleased]section ofCHANGELOG.mdat the branch tip. Only breaking or user-visible changes are listed; additive JSON keys, internal staleness self-heals, and automatic index rebuilds are omitted.Breaking (automatic index rebuild required on first query after upgrade):
TEMPORAL_DATA_VERSIONself-heal rebuild per root (one slow query, then fast). Hot and risky scores will differ from pre-🟠 P1: Temporal layer counts commits first-parent-only — hot/risky ~3× undercounted, fix-risk inverted for branch work, contradicts skim's own heatmap #407 values on branch-heavy repositories.-i,-w) are now rejected with a clear error. Combining a text query with an action flag (for exampleskim search foo --rebuild) is now a hard error. Use--to pass dash-leading terms as query text.Removed:
skim search indexas a build command is gone. Builds use--build,--rebuild, or--update. A bareskim search indexnow runs a lexical query for the word "index".Added:
--offset Npagination across all query arms, including standalone temporal.--phrase --near Ncomposition (ordered and span-bounded simultaneously).--end-of-flags separator: everything after--is treated as literal query text.ast_coveragein--ast --jsonand--stats --jsonwhen files exceed the 1 MiB AST cap.git_head_state,temporal_state, andstalenesskeys in--stats --json.degradedarray in--jsonoutput when a temporal arm cannot be served.N result(s) for "query" in Tms.Changed:
--blast-radiustemporal axis ranks by Jaccard co-change strength, seed first. Files with zero co-change partners no longer appear as their own result.--root <subdirectory>adopts the enclosing repository's HEAD and scopes temporal rows to the subtree.--install-hooks/--remove-hooksin a linked worktree now route to the sharedcommondir/hooksdirectory and disclose that scope on stderr.Verification
CI: The tip commit is
0f0d0bd; CI runs 33979208549 and 33979206924 were in progress at the time of writing, see the Checks tab.Local gates (cold cache, Rust 1.98, on branch tip
0f0d0bd):cargo fmt -- --check: cleancargo clippy --all-targets --all-features -- -D warningson Rust 1.98: cleancargo nextest run -p rskim-search: 1106 passed, 2 skippedcargo nextest run -p rskim --all-targets: 5427 passed (4 slow), 3 skippedcargo test -p rskim-search --docandcargo test -p rskim --doc: cleanIndependent wave-report verifications:
Two read-only verification agents independently audited the engine-produced wave reports against the gate journal artifacts and source code at the merge commits:
.devflow/docs/waves/407-409-temporal-correctness/2026-09-03_1500/wave-report-407-verification.mdaudited 🟠 P1: Temporal layer counts commits first-parent-only — hot/risky ~3× undercounted, fix-risk inverted for branch work, contradicts skim's own heatmap #407 (31 commits, 6 gate sweeps). Headline: implementation verifies clean, post-merge gate sweep fully green at58a7967(1104 rskim-search + 5049 rskim, 4 skipped). The report itself had several overclaims (a Gate-1 FAIL at825fbaathat was fixed before the merge commit, AC criteria whose test assertions were narrowed to match implementation). The post-merge state is clean..devflow/docs/waves/407-409-temporal-correctness/2026-09-03_1500/wave-report-409-verification.mdaudited 🟠 P1: Composite --weights temporal axis on --blast-radius is not co-change strength — 0,0,1 collapses to alphabetical order and buries the strongest partner #409 (23 commits, 5 gate sweeps). Headline: implementation verifies clean, all five gate sweeps on the ticket branch are green at tipf91f17e(1106 rskim-search + 5069 rskim, 2 skipped). The report undercounted commits (5 vs 23), omitted 48 failed agent rows from two permanently-failed fix batches, and did not qualify Gate-2 evidence that was 14 commits stale. The code-level spot-checks all pass. (The conflict oncli_temporal_first_parent.rsresolved cleanly in the integration merge at64b1926.)Both verifications are on record in the repo tree under
.devflow/docs/waves/. The.devflow/docs/tree is listed in.gitignore; the paths are local to the clone.ADR-007 round-4 dog-food campaign (promotion gate):
Six corpora, 136 probe rows, 5 findings confirmed: 0 P0, 2 P1, 3 P2. All 5 fixed in 5 commits (
c2eac80,b545577,718adff,229c108,b3f3a72) and re-verified on a freshly built binary (skim 2.11.0 (b3f3a72)) across 9 verification checks, all PASS.Full synthesis:
.devflow/docs/reviews/dogfood-2026-09/synthesis.md(local path, gitignored).Fix re-verification:
.devflow/docs/reviews/dogfood-2026-09/reverify-fixes-results.md(local path, gitignored).Known gaps and deferred work
--depth 1clone is a merge commit (so the merge-skipping DAG walk sees zero non-merge commits) was not exercised by either shallow corpus. The standard shallow-clone path (non-merge HEAD) is covered.--buildand--rebuildreport no temporal outcome withoutSKIM_DEBUG=1, add atemporal: ok | empty | corrupt-recovered | refusedtoken to the summary line #486, search: symlink defense-in-depth for temporal/heatmap containment guard (#408 follow-up) #455, Wave 4 / Search: make candidate-vs-match explicit in SearchLayer::search() return type (deferred from #355) #365, Wave 4 / Search: regenerate trigram_weights.rs from the full ~118k-file corpus for IDF selectivity (deferred from #355) #366.heatmap --sincedate-format inconsistency: observed during the campaign but not filed (accepted as a known rough edge, not affecting search correctness).#203(Wave 4e golden test repo): not on this branch, not blocking.Merge notes
Two reconciles of
origin/maininto the branch: the first at67cdaffbrought in v2.11.0, #428, #385, #488 and 14 additional main-branch fixes; the second brought in #508. No history rewrite occurred. Squash-merge expected per ADR-005; the merge decision is the author's.The working tree carries an in-progress merge of
origin/mainintowave/wave4-searchby a concurrent agent. That merge touchescli_temporal_first_parent.rsonly (the same file that conflicted in the #409 integration) and is not part of this PR.Housekeeping at merge time
ref: refs/../../../outside-shaescapes the git dir and the escaped SHA is persisted (tracking for #413 Step 1) #482 (shipped as part of 🟠 P1: Linked git worktrees — HEAD symbolic-ref resolution fails, temporal layer silently dead (stats 'git HEAD: (none)', temporal.db never built) #413; the symbolic-ref path-traversal guard is in59cbd2b).Relates #174 (north star tracking issue, stays open).
Closes #198, #200, #201, #202, #286, #287, #289, #290.
Closes #355, #356, #357, #358, #364.
Closes #372, #373, #374, #375, #376, #377, #378, #379, #380, #381.
Closes #393, #394, #395, #396, #397, #399, #400, #402, #403, #404, #405, #408, #411, #412.
Closes #413, #414, #407, #409.
Closes #175, #176, #177, #178, #179, #180, #181, #182, #183, #184, #185, #186, #187, #188, #189, #190, #191, #192, #193, #194, #195, #196, #197, #199.