Repository navigation
feat(search): yield one hit per unit colgrep matched in a file - #18
Merged
Merged
Conversation
… observation A search hit on a long markdown file shows the file's first lines, not the lines that matched: colgrep's --json carries no match location, so hit_from_raw takes the unit's first snippet_lines. The observation records the probes and three fixes (A in this server, B and C upstream). The Side Jobs mission reminder sj-colgrep-match-lines cites this report as its spec, and a worktree sees only commits, so it has to be on main before an agent picks the mission up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colgrep collapses results to one per file (collapse_by_file,
colgrep/src/index/mod.rs). Since 1.8 each result also lists its matched
units in `matches: [{line, end_line, name, score}]`, best first, whenever
two or more matched. search showed only the best unit, so on markdown
indexed by section (fork 1.8.5+cs.1) an answer in a note's second-best
section was hidden and agents read the whole file. On a copy of
Murmuration's __statera__, "what did the PI decide about w05-c1's
escalation to switch to a subagent worker" ranks wave-05.md first with
sections (162-180), (105-156), ...; the answer, line 138, is in 105-156.
search now emits one SearchHit per matched unit with its own hit_id and
score, ranks all units by score across files, and lets `limit` count
units: colgrep's `-k limit` files always hold the top `limit` units, so
the server cuts the rest and sets `truncated`. On the probe above,
105-156 is hit 15 of 25 (inside the default limit of 15),
location_verified, and expand returns its 52 lines including line 138.
The other units come without `code`. Their snippet and code are sliced
from the file text already read for the leader, so no extra I/O; their
ranges count as verified only when the leader was located exactly at
matches[0]'s range. They carry no qualified_name, unit_type or
signature, and the text block drops the empty separators. find_files
counts each matched unit in `hits` and names them in `top_units`.
Output without `matches` (colgrep 1.6, or a single unit) is unchanged.
fake_colgrep.py gains FAKE_COLGREP_MATCHES=1, folding per-unit fixture
hits per file the way 1.8 does. The four new tests through
Client(build()) failed against the previous tools_search.py.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of c4c291a: the one-hit-per-unit shape is right, but three readers of the matches wire shape, three location bugs and input-proportional work call for a rework on this branch. One implementer leaf, one coordinator leaf (doctor minimum 1.8.4, docs edited in place), one adversarial review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…by path test_packaging's local-path guard rejects machine paths in the tree; the brief carries them instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colgrep 1.8.4 (upstream 1af6b16) is the first release whose --json lists every matched unit of a file in `matches`; search now builds one hit per unit from it. An older binary still works, but search sees only each file's best unit and locates it by reading the whole file, so the minimum is soft: doctor reports it as a problem with an upgrade hint rather than search refusing to run. The hint is version-neutral and does not suggest `colgrep update`, which reinstalls the official build over a fork. The fake now reports 1.8.5 (the version its fixtures model), and the README's requirement cell moves from 1.6 to 1.8.4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c4c291a appended a paragraph to the guide and a clause to the skill. Both were defensive accretion: the paragraph restated the hit shape and listed fields structured_content already shows, and the skill already says to expand a handful of hits and defers to the guide for reading them, while costing tokens on every trigger. The skill goes back to main's text. In the guide, the existing "A hit is one unit" sentence gains one clause (one hit per matched unit, ranked across files); the location_verified paragraph is rewritten to state the rule the server now applies (a file's best unit checked against its source, the others once it sits at colgrep's range and the file is unchanged since this search refreshed the index; an unverified non-best unit has no snippet) and drops the colgrep 1.6 history; the truncated sentence says only what the server can know, its own cut to limit or the text budget. AGENTS.md gains the OBS-M legend row the search docstrings cite. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ches colgrep returns one result per file and, since 1.8.4, lists each matched unit in `matches`, best first, when two or more matched. Two functions re-derived that shape and diverged: search deduped on range, find_files did not, so a duplicated range counted twice there. colgrep's own idiom is one normaliser, SearchResult::unit_matches() in index/mod.rs. adapter.unit_matches is now the only reader of `matches`: leader first, one entry per (line, end_line) at the best score as absorb_matches does, non-leaders stable-sorted by score so a cut is a prefix, ranges outside 1 <= line <= end_line dropped, and a fall-back to the result's own unit when `matches` is absent or lone. find_files counts those units and no longer lists empty names (documents, raw code) in top_units. The fake drops FAKE_COLGREP_MATCHES and its re-implementation of collapse_by_file: it answers a fixture as written, like its other knobs. hits_small.json and the matches tests move to literal 1.8-shaped results. The duplicate-range test through Client(build()) fails on c4c291a through find_files (3 hits counted for 2 units). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c4c291a built a file's best unit and its siblings in two places that disagreed, and four things went wrong. An unlocatable best unit fell back to unit.line..end_line, the range colgrep widens over every matched unit, so its hit_id overlapped all its siblings; it now falls back to its own matches[0] range. A sibling was trusted whenever the best unit sat at its own range, so an edit between the two moved the sibling while it kept snippet and code sliced at the old lines, the right name over the wrong text. A sibling with no name (colgrep names no document or raw-code unit) passed that test too. And `name` mixed unit.name with colgrep's display names. A sibling is now trusted only when the best unit was located exactly at its own range and the last dotted segment of its display name occurs in the range's first line (so `## v1.8 notes` passes). An unnamed one never is. An untrusted sibling gets no snippet and no code. One private builder serves every sibling, and every hit of a file takes colgrep's display name; the best unit keeps qualified_name, unit_type and signature. The location_verified description states this rule. A hit whose located range equals a sibling's reported one is no longer dropped: unit_matches already collapses colgrep's own duplicates, and the case left is a moved file, whose siblings are unverified anyway. Contract tests through Client(build()) pin the edit below the best unit, the unlocatable best unit, the unnamed sibling, include_code on a verified sibling, a one-entry matches and a dotted heading; test_render pins the head of a signature-less hit (no trailing separator), which c4c291a changed unpinned. The edit-below, unlocatable and unnamed tests fail on c4c291a; the other four pass there and pin what must not regress. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…span
search built a SearchHit for every unit colgrep sent, then cut to limit,
and read every hit file whole, split it twice (locate_unit, then the
sibling slices) and scanned it for each leader's first line. The work
grew with the files and units colgrep returned, not with the hits shown.
_do_search now stable-sorts every unit by score (ties by colgrep's result
and unit order) and cuts to limit before anything is read or built. A
file's units are best first (unit_matches), so its survivors are a prefix
and its best unit is among them. _plan_reads names the ranges the
survivors need, _read_units reads them in one islice pass per file in
asyncio.to_thread with one gather across files, and the pure builders
take file -> {range: lines} (FileLines) instead of whole texts.
A leader is verified by comparing the lines at colgrep's own range with
its code (locate.span_holds, trailing whitespace ignored through the
same rstripped helper locate_unit uses). Only when that fails (a file
edited since indexing, or a pre-1.8.4 colgrep whose range is widened) is
the whole file read, once, and locate_unit run. A whitespace-only edit
since indexing now verifies. Spans split on \n alone, as colgrep counts
lines, rather than str.splitlines (form feed, U+2028).
Measured through Client(build()) with a stub adapter, timing _do_search
alone, median of 5 invocations of 5 calls each, 15 markdown files of
2,000 lines with 30 matched sections each (450 units):
| search | before (2afb0fa) | after |
|:------------------|-----------------:|--------:|
| limit=15 | 5.20 ms | 2.13 ms |
| limit=None | 5.10 ms | 4.50 ms |
An exhaustive listing builds every unit either way, so it gains only
from the cheaper reads. A first version that stripped every span line in
Python took 9.5 ms there; the spans now keep their newline and are
stripped where used.
Contract tests: a span-verified leader never calls Path.read_text; an
edited file is read whole exactly once to relocate its leader, and the
unedited file beside it not at all; the cut reads only the survivors'
ranges; _read_units opens a file once for all its spans; the off-loop
test now spies on open (the happy path no longer calls read_text); expand
returns CRLF lines without terminators. On c4c291a the span-verified,
edited-file (it read both files whole) and off-loop tests fail; the
expand test passes there and pins what must not regress; the _read_units
and cut tests exercise the new API.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
search now yields one hit per matched unit, so an agent expanding the hits that matter passes several hit_ids of one file as the common case, and expand opened and islice-d the file once per id. expand groups the parsed ids by file and makes one _read_spans pass per file, in asyncio.to_thread with one gather across files, then answers in input order. Overlapping and non-contiguous spans share the pass: it reads the prefix up to the largest end_line once and slices each span from it. A malformed id still yields its own error unit, and an OSError is attached to every id of that file. Measured through Client(build()), the whole expand call, median of 5 invocations of 5 calls each, 5 ids of one 2,000-line markdown file (about 66 lines each, scratch corpus): | expand, 5 ids of one file | before (86d3254) | after | |:--------------------------|-----------------:|--------:| | whole call | 0.76 ms | 0.42 ms | The contract test gives three ids of one file, out of order and one overlapping, and checks the spans in input order and a single open; it fails on c4c291a (three opens). A second pins that malformed ids and an unreadable file keep their errors per id under grouping. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…efresh The verifier broke the wording check that decided whether a non-best unit's range could be trusted. A name ending in a dot (colgrep keeps `Title doc..Next steps.`) has an empty last dotted segment, which occurs in every line; duplicate headings and a name merely mentioned in a line passed; and the continuation chunks of a long markdown section, whose first line is body text, failed closed: on an unedited statera copy 13 of 110 hits lost their snippet and a LOCATION_UNVERIFIED note fired on every query, where c4c291a had none. The check is replaced by a guarantee. Before a search without --no-update, colgrep re-checks every file's mtime and size and re-indexes the changed ones (commands/search.rs load_index, compute_update_plan in index/mod.rs), so a file not modified since the call began was indexed as it is, and colgrep's ranges for it are exact. A non-best unit is trusted iff the best was span-verified exactly at its own range, the call did not pass skip_index_update, and the file's st_mtime_ns, read with fstat on the handle _read_spans_from already opens, is at most the time_ns taken just before colgrep was spawned. The builder stays I/O-free; the mtime travels in FileLines. A unit's name no longer matters, so an unnamed one is trusted like any other. An untrusted unit still has no snippet and no code, and the location_verified description states the rule. One premise of the intended rule is false and is left out: colgrep prints "Index is being updated by another process, searching existing index..." only when !quiet, and `--json` sets quiet (search.rs `let quiet = json || files_only`, passed to load_index). Probed on a scratch corpus with the index lock held: `search --json` exits 0 with empty stderr, the same call without --json prints the line. So a search that raced another indexer cannot be told apart, and its other units are still trusted. The window is another process holding the lock; this server serialises its own calls per project. Tests through Client(build()): a file modified after the search began and a skip_index_update search both leave the other units unverified with no snippet or code; an unedited file whose units are a mid-body chunk, a trailing-dot name and an unnamed range is verified in full with snippets and no LOCATION_UNVERIFIED note. They replace the name-check tests. The unedited-file and skip_index_update tests fail on 2bd2a8c. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colgrep numbers lines by \n, and expand reads a hit's range the same way, but locate_unit and search's code line counts still used str.splitlines, which also breaks on form feed, vertical tab, \x1c-\x1e, NEL and U+2028/9. A file holding one such character got a located range shifted against expand's, with location_verified=True: the verifier's form-feed case returned 6-8 for a unit expand reads at 5-7. The previous commit's claim that the exact-span path fixed this held only for that path. locate.split_lines breaks on \n and drops only the final empty element; locate_unit (file text and code) and the code line counts in tools_search use it, so every range agrees with expand. A \r stays on its line, since text mode already folds it out of file text and colgrep's code can carry it: locate_unit now compares the first line and the single-candidate block with trailing whitespace ignored, as span_holds and the multi-line disambiguation already did, and a hit's snippet drops a trailing \r from each code line as splitlines did. A CRLF unit still verifies. Contract tests through Client(build()): a stale file with a form feed above the unit, where the hit_id range must be the one expand returns as the unit (fails on 2bd2a8c), and a CRLF unit located after a shift (passes there and pins the \r handling). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colgrep collapses its results to one per file inside one search path (collapse_by_file, called per path from search_hybrid_with_embedding); cmd_search then extends the per-path results, sorts and takes top_k with no further collapse (commands/search.rs). A search over nested paths therefore returns a file once per path, and every unit in both results came out twice: duplicate hit_ids from search, and duplicate counts in find_files. After the stable sort and before the cut, _do_search keeps the first, best-scored sighting of each (file, line, end_line, name) and stops scanning once limit is passed, so the cut stays proportional to limit. find_files' fold counts a unit once the same way, and keeps its limit*3 over-fetch, which the same overlap makes necessary. The name is in the key, beyond the ruled (file, line, end_line): before 1.8.4 colgrep's per-unit ranges were unreliable enough (R05 D1) for two units of a file to report one range, and test_errors' three hits of one file at 7-9 (vanished_0..2) is that shape; keyed on the range alone they collapsed into one hit. Benchmark unchanged (stub adapter, 450 units, median of 5): limit=15 2.17 ms, limit=None 4.53 ms, against 2.13 and 4.50 before. The contract test returns notes.md twice, the second time with another set of units sharing one, and expects each unit once from search and from find_files; it fails on 2bd2a8c. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…a score tie absorb_matches sorts a file's matched units by score and then by line ascending (index/mod.rs), so when the best two units tie, matches[0] is the lower-line one even if `unit`, whose `code` colgrep sends, is the other. unit_matches took matches[0] as the leader on faith: the leader hit then carried one unit's name over the other's text, repeated the other unit's range as its sibling's, and a cut to limit=1 kept the wrong unit (the verifier's tie case). Among the entries tied with the first, the leader is now the one whose line count equals the line count of unit.code (split on \n) and whose name equals the unit's display name, parent_class + "." + name when a parent class is set, empty for document and raw_code units, as UnitMatch::of builds it; with no such entry, or no code, it stays matches[0]. The rest keep their score order. The contract test gives a tie where matches[0] is Logistics and unit is Escalation, and expects Escalation as the verified leader with its qualified_name, every unit listed once and no duplicate hit_id; it fails on 2bd2a8c. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e normaliser, units built after the cut, verified ranges) The match_units unit_pipeline leaf: adapter.unit_matches is the only reader of colgrep's matches; search ranks and cuts units before building any, reads only the surviving spans, verifies a file's best unit by its exact span and trusts the others through this call's index refresh; expand reads each file once. Verified by an adversarial review and its recheck, including the real binary on a scratch notes corpus (0 of 110 hits unverified, 75 of 75 best units span-verified). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Its --version, fixtures and matches shape are 1.8 since the match_units rework; the header still said 1.6. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
All three nodes are done. unit_pipeline step 5 (find_files -k limit) was skipped on evidence: colgrep collapses per search path, then concatenates and takes top-k (commands/search.rs:543-613), so -k limit could return fewer than limit files. Open gates: PR #18 CI, and the literal 'every new test fails on c4c291a' gate, which holds for the behaviour tests while the regression pins pass there by design. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e cycle's retrospective The rework left the server inferring 35 of 110 hits' locations, because colgrep sends no content for a file's non-best units. The findings report ranks four upstream asks (a per-unit hash in matches, the best unit kept first on ties, the index-update outcome under --json, a merge across search paths), the gaps each closes and what not to ask, so the work can be picked up through the CrackingShells/next-plaid fork. The knowledge-transfer report records why both trust-rule designs were wrong and what changes next cycle; the observation hands the dirtree-rdm add-suffix defect to its own repository. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.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
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.
Why
colgrep returns one result per file per search path. Since 1.8.4 each result also carries
matches: [{line, end_line, name, score}], one entry per matched unit, best first. The field appears only when two or more units matched (SearchResult.matchesandcollapse_by_fileincolgrep/src/index/mod.rs).searchused to show only the best unit, so an answer in a note's second-best section was hidden and agents read the whole file instead (OBS-M).What changes for a client
searchreturns one hit per matched unit. Each hit has its ownhit_id, all hits are ranked by score across files, andlimitcounts units.truncatedis set when the server's own cut tolimitdropped units.location_verified:skip_index_updateis off and the file's ns-mtime is no later than the spawn time; colgrep re-indexes every changed file before it searches.matchesis present, every hit of the file uses colgrep's display name.\nalone, so they agree withexpand.codecolgrep actually sent.find_filescounts and names a file's matched units. It keeps itslimit*3over-fetch, because colgrep collapses results per path, not globally.doctorreports colgrep older than 1.8.4 as a problem (a soft minimum: older binaries still work, just more slowly). The README requirement now says ≥ 1.8.4.skills/is unchanged frommain, andAGENTS.mdgains theOBS-Mlegend row.Design
adapter.unit_matchesis the only reader ofmatches. It returns leader-first matches, deduped and validated, and treats output withoutmatchesas one unit.searchranks and cuts before building any hit. It then reads only the surviving spans, in oneislicepass per file (off the event loop).expandreads each file once, however many ids it is given.Measured (stub adapter, median of 5; 15 markdown files × 2,000 lines, 30 matched sections each)
searchpost-adapter, limit=15searchpost-adapter, limit=Noneexpand, 5 ids of one fileEvidence
The real binary (colgrep 1.8.5+cs.1) ran on a scratch copy of Murmuration's
__statera__: 5 queries, 110 hits.wave-05.md:105-156is verified, andexpandreturns line 138.An adversarial review broke the first trust rule, which matched a unit's name against the first line of its range. That rule failed on continuation chunks, trailing-dot headings and duplicate headings. The review also caught form-feed line counting, duplicates from overlapping paths, and score ties. All of these are fixed, and a recheck confirmed the fixes.
Known limits (accepted)
--json), can leave later units trusted against a stale index.Commits
__roadmap__/match_units/: the rework, as one implementer leaf merged with--no-ff, plus the doctor and docs commits.Gates:
uv run pytest(336 passed),ruff check,ruff format --checkandcz check --rev-range main..HEADall pass.🤖 Generated with Claude Code