feat(search): query-aware truncation#147
Merged
Merged
Conversation
When an expanded body exceeds the smart-truncate threshold, select_diverse_lines now boosts lines containing the searched symbol (and their +/-1 neighbours) so the lines the agent actually asked about survive truncation instead of being dropped for generically important ones. The single production call site already had the matched symbol (m.def_name) in scope. Replaces the dead _lang param with the query slot (the design's intent), removing end-to-end dead plumbing. None/empty query is byte-identical to before. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Query-aware truncation passed `def_name` as the symbol to boost. For
impl/implements matches `def_name` is the rendered label ("impl Iterator
for Counter" / "Type implements Trait"), which never appears in the
body — so the boost was a no-op for that whole class of matches. Route
the searched symbol via a new `boost_query` helper: `impl_target` (the
trait/interface the user searched for) for impl matches, falling back to
`def_name` for plain definitions.
Also fold query-match detection into the structural-scoring pass so each
body line is read once, dropping the redundant second `lines.get` and the
always-true `is_some_and` guard. Empty queries normalise to `None` up
front, keeping the no-query path byte-identical.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jahala
force-pushed
the
feat/qtrunc-query-aware-truncation
branch
from
2565c12 to
344446d
Compare
paulnsorensen
added a commit
to paulnsorensen/tilth
that referenced
this pull request
Jul 4, 2026
…et alloc, Bash grammar, tilth_savings) (#102) * feat(read): never-worse outline gate (jahala#146) (cherry picked from commit c4803af) * feat(grok): auto-expand thin wrappers (jahala#149) (cherry picked from commit 3f40056) * fix(grok): size-cap thin-wrapper delegate reads; harden the predicate The cross-file delegate read in thin-wrapper auto-expansion called fs::read_to_string with no size bound, so a wrapper resolving to a huge or generated callee file would slurp the whole thing into memory. Route the read through a new `read_delegate_content` helper that reuses the already-loaded target content for same-file callees and caps cross-file reads at the shared source-file bound (bloom_walk::MAX_FILE_SIZE). The cap is a parameter, so it is unit-tested with a tiny fixture instead of a 500KB one. Also harden the predicate: - Capture the sole internal callee pre-cap (`lone_internal_callee`) so expansion is correct regardless of `caps.max_callees`, replacing the post-cap `callees_internal.first()` and its misleading "safety" note. - Add a `start_line > 0` guard so a degenerate/unresolved target cannot produce an empty delegate body, mirroring `slice_body`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 9df5120) * ci(release): dual-publish npm wrapper as @plotplot/tilth, best-effort (jahala#144) (cherry picked from commit e9155c1) * feat(search): query-aware truncation (jahala#147) (cherry picked from commit 76abb65) * feat(search): value-based budget allocation (rate-distortion) (jahala#150) (cherry picked from commit 66ae97e) * fix(sync): dedup read_delegate_content after grok cherry-picks The 9df5120 size-cap pick applied cleanly but produced a duplicate definition of read_delegate_content alongside 3f40056's copy. Remove the redundant second definition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(lang): add Bash tree-sitter support (jahala#151) (cherry picked from commit bc2d7e8) * fix(sync): thread edit_mode into format_matches test call site The value-based budget pick (66ae97e) added a segments-collecting test that calls format_matches, but our fork's format_matches also takes an edit_mode arg. Pass false at that call site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(mcp): runtime token-savings tool (tilth_savings) (jahala#148) (cherry picked from commit 63553eb) * feat(mcp): add readOnlyHint annotations to tool definitions Read-only MCP clients (e.g. Claude Code plan mode) gate any tool that does not declare readOnlyHint, prompting on every call. Mark the read tools read-only and tilth_write as not. (cherry picked from commit 8196d27) * fix(callers): 2nd-hop analysis + scaled budget + unified headers for multi-target Addresses 3 unresolved PR review findings on the comma-separated multi-target callers search: - HIGH: the multi-target path omitted the single-target path's "Adaptive 2nd-hop impact analysis" entirely, so callers("a,b") returned strictly less than two separate callers("a") / callers("b") calls. Extracted the block into write_second_hop_impact (previously inline in search_callers_expanded) so both paths call the same code per target bucket. - MED: BATCH_EARLY_QUIT (a walk-wide raw-match budget shared across every target in the batch walk) was not scaled by target count, so an earlier hit-rich target could exhaust the walk before a later, rarer target's files were visited. Added scaled_batch_quit, a pure function multiplying the single-target budget by target count (bounded — dispatch already caps multi-target queries at 5). The early-quit mechanism itself is a coarse heuristic and a candidate for future removal/replacement; this is a minimal parity fix, not an investment in it. - MED: header format diverged between single (`# Callers of "foo" in <scope> — N call site(s)` / `## path:line`) and multi (`## callers of "foo"` / `### path:line`) purely on comma-presence. Extracted write_caller_bucket so both paths render the identical shape; a bucket inside a multi-target query is now byte-identical to what a lone single-target search of that symbol would produce. Both extractions move existing logic verbatim rather than reimplementing it — search_callers_expanded's behavior is unchanged except for the mechanical format!->writeln! swap clippy requires when appending to a shared &mut String instead of constructing the first String. Tests: scaled_batch_quit_multiplies_by_target_count, scaled_batch_quit_treats_zero_targets_as_one (direct unit tests on the pure scaling function — deterministic, unlike asserting on the parallel walker's actual starvation behavior), callers_multi_target_includes_second_hop_impact_per_bucket, callers_multi_target_header_matches_single_target_shape, callers_multi_target_later_target_not_starved_by_hit_rich_earlier_target (scenario-level guard), plus two pre-existing tests updated for the corrected header casing (Callers vs callers). (cherry picked from commit 8e6c1bd) * fix(benchmark): correct three ground-truth strings per grading audit trustedCIDRs -> isTrustedProxy in gin_client_ip: the field is real but sits one call-depth past ClientIP()'s own body; isTrustedProxy is the method ClientIP() actually calls, same signal, zero-hop-deeper. Drop view.js from express_render_chain and express_app_render: the file is imported via require('./view') (Node's extensionless convention) and never appears as literal text in source, so the string was unwinnable regardless of answer quality. Applied exactly as proposed in docs/research/grading-audit-2026-07.md §6 — verified against the pinned commit via git grep, cross-checked by an independent manual read, zero flips on the three flagship hard tasks. rg_search_dispatch/glue.rs deliberately left untouched: same bug class, but flagged as a maintainer decision, not auto-applied, since its failures are genuine navigation misses, not phrasing gaps. (cherry picked from commit f195bb9) * fix(search): suppress outline preview line when a fence will reprint it format_single_match always printed the raw "→ [line] text" preview before checking whether expansion would run. expand_match's fence range always contains m.line (def_range starts at m.line for definitions; the ±10 fallback for def_range: None / usages trivially contains it too), so whenever expansion actually fires the preview line is a byte-identical duplicate of a line inside the fence below. Repro: searching a <50-line file for a definition with no doc comment (e.g. TilthError::exit_code) printed the matched line twice — once as the "→ [31] pub fn exit_code(&self) -> i32 {" preview, once again as line 31 inside the whole-file expansion fence. Hoists the dedup/skip decision (previously computed after the preview) into a `fence_will_follow` predicate checked before writing either raw preview line (small-file branch and the no-outline fallback). The structural outline_context branch is untouched — it renders neighboring entries' signatures, not the matched line's own source, so it doesn't duplicate the fence. (cherry picked from commit d791449) * fix(search): thread caller budget into fit_to_budget — value-based truncation now engages below 24k Both fit_to_budget call sites (search/mod.rs format_search_result and search_multi_symbol_expanded) hardcoded crate::budget::DEFAULT_BUDGET (24,000) instead of the caller's real budget, so value-based match selection only ever activated when the unconstrained render exceeded 24k tokens — every realistic caller budget silently fell through to budget::apply's positional tail-cut instead (docs/research/tilth-audit-2026-07-logic.md §3). Threads budget: Option<u64> through the four *_expanded search functions and format_search_result, defaulting to DEFAULT_BUDGET via unwrap_or when None so the no-budget path stays byte-identical. Wires the already-extracted real budget at both call sites that have one: MCP's tool_search (mcp/tools/search.rs) and the CLI's run_inner (lib.rs), whose outer budget::apply pass is now a no-op once fit_to_budget already selected within the same real budget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 2f596b0) * refactor(map): share the gitignore/hidden/skip-dirs walker policy with search map.rs built its own inline WalkBuilder duplicating search::walker()'s ignore/hidden/git_* flags and SKIP_DIRS filter_entry — but silently missing same_file_system(true) (stop at NFS/external mount boundaries), a guard every other walker in this codebase carries (search::walker, search::find_basename_fallback). Diffed the two policies: the ignore/hidden/gitignore flags were already identical between map and search (no deliberate divergence to document there). The only structural differences are max_depth (map-specific, bounds the tree to the CLI depth arg) and sequential vs parallel execution (map incrementally builds a BTreeMap per-entry; search fans out across threads) — both stay as-is, since collapsing map onto the parallel model would be a control-flow rewrite, not a policy fix. Extracted the shared flags into search::base_walk_builder(scope) -> WalkBuilder. walker() adds .threads()+overrides+.build_parallel(); map::generate adds .max_depth()+.build(). map now inherits same_file_system(true) for free. (cherry picked from commit 21c4faf) * test: add direct unit coverage for treesitter weight/name extraction and sibling resolution lang/treesitter.rs::definition_weight and ::extract_definition_name had zero direct tests — only exercised transitively through search/symbol.rs's larger integration tests. Added: - definition_weight_covers_every_tier: one assertion per weight tier (100/90/80/70/60/40/30/default-50), spot-checking a node kind from each source language shape the match arms group together. - Five extract_definition_name cases against real parsed nodes (Rust function_item, Python class_definition, TS export_statement unwrapping, TS lexical_declaration's variable_declarator walk, and the None fallthrough for impl_item, which has no name/identifier/ declarator field and isn't handled by any special-cased branch). search/siblings.rs had no test module at all. Added 8 tests for the pure resolution logic named in the brief (resolve_siblings, find_parent_entry) — signature-copy, signature-fallback-to-name, unmatched-name skip, function-before-field ordering, alphabetical tiebreak, MAX_SIBLINGS truncation, and parent-lookup hit/miss. Scoped to resolution only, not the tree-sitter query extraction (extract_sibling_references) — that's a much larger, per-language surface the brief didn't name. Verified both suites have teeth: mutated definition_weight's impl_item/object_declaration arm (90->91) and resolve_siblings' sort comparator (b_is_fn.cmp(&a_is_fn) -> a_is_fn.cmp(&b_is_fn)), confirmed each mutation failed its respective test, then reverted. (cherry picked from commit be6195d) * fix(sync): resolve clippy findings from upstream ports Apply clippy's suggested rewrites for uninlined_format_args, format_push_string (writeln! to avoid write_with_newline on the trailing newline), format_collect (fold), and doc_markdown backticks. Delete unused #[cfg(test)] Session::reads_count (leftover from the tilth_savings port; not present or used upstream). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(sync): derive read view-meta from actual output, not would_outline prediction The never-worse outline gate (OGATE, from upstream c4803af) can return full content for a file that would_outline predicts as outlined, which left the meta header claiming view:"outline" over a [full] body and prompted a pointless re-read. Key the meta off the emitted view marker. Also rebuild the large-structured-read test fixture with values longer than the outline's 40-char preview cap, so the keys outline actually compresses and the gate stays off — same fixture adjustment upstream made to its own alias test in c4803af. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: jahala <jahala@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Martin Wedvich <wedvich@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
Makes
select_diverse_lines(the smart-truncation line selector) query-aware: lines containing the searched symbol get a score boost (and their ±1 neighbours a smaller one), so they survive truncation.Why
The selector kept generically "important" lines (signatures, control flow, calls) but was blind to what the agent searched for — so the very lines mentioning the target symbol could be truncated away, forcing another search. There's a single production call site, and the matched symbol (
m.def_name) was already in scope there.How
_langparam withquery: Option<&str>(the slot the design always intended) — also removes end-to-end dead plumbing, since the call site no longer extractslangjust to ignore it.queryisSomeand non-empty):QUERY_MATCH_BOOST(50) on lines containing the symbol,QUERY_PROXIMITY_BOOST(5) on ±1 neighbours, atop the structural score. Integer, deterministic,saturating_add.None/empty query → pass 2 skipped → byte-identical to before.Tests
query_boost_rescues_needle_line— differential: a low-score line carrying the symbol is dropped withNonebut kept withSome(it sits past the 40-line keep-window; score 1→51).query_boost_preserves_neighbour— the ±1 neighbour survives via the proximity boost.empty_query_behaves_like_none—Some("")≡None.472 tests pass ·
clippy -D warningsclean ·fmtclean.🤖 Generated with Claude Code