hyp query grep: the full search stack, integrated on master (LLP 0265 T4-T7) by bgmcmullen · Pull Request #984 · hyparam/hypaware

bgmcmullen · 2026-08-21T18:00:08Z

What this is

The whole hyp query grep feature (LLP 0265 T4-T7) as one branch against current master, so it can be checked out and exercised locally and merged with a single click. It supersedes the four-PR stack #951 / #952 / #953 / #954, which carried the same work but could not be merged in sequence: this repo squash-merges, so each merge would have re-proposed its parent's changes to the branches behind it.

Every review those four PRs received is included here, including neutral's fixes on each.

Trying it locally

git fetch && git checkout grep/integration && npm install # the lockfile moved; see below
hyp query grep "some text you remember"
hyp query grep "needle" --session-id <id> --from 2026-08-01 --limit 5
hyp cache status # ends with the grep index coverage line
npm run smoke -- query_grep_roundtrip

npm install matters: master bumped squirreling to 0.16.1 and a stale node_modules fails typecheck with ~75 errors in union-source / sql that have nothing to do with grep.

What it adds

The search service (src/core/search/grep_service.js): one newest-first walk over the cache's live data files; each file served either through its hypgrep .index.parquet sidecar or by a brute scan of the nine allowlisted columns. Purge-correct (position deletes filtered on both tiers, so a stale sidecar cannot resurrect a purged row) and visibility-correct (LLP 0105, via the same predicate the SQL path uses).
The verb (grep_verb.js, registered in CORE_VERBS): hyp query grep plus the grep_search MCP tool. The schema is wire-compatible with the server's own grep_search, so --remote <target> reaches a server's archive-backed search with no server-side work.
Sidecar builds at maintenance (sidecar_build.js + worker pair): compaction finalizes a file, then an index is built for it in a worker thread. Sidecar existence is the completion marker, the publish is write-then-rename, and a file whose build keeps failing is quarantined after three attempts and served by the scan tier forever after.
Surfaces and proof: a grep index: N of M data files indexed line on hyp cache status, the grep section in both copies of the hypaware-query skill, and the query_grep_roundtrip hermetic smoke (added to the release battery).

Integration fixes this branch carries

Two things only showed up once the four branches sat together on current master:

sidecarPathFor moved modules during Sidecar builds at maintenance: compaction finalizes a file, the index follows (LLP 0265 T6) #953's review, while a Surfaces and proof: status coverage, the skill learns grep, and the roundtrip smoke (LLP 0265 T7) #954 review commit still imported it from its old home. Typecheck error and every maintenance test red.
Master's D1 short-flag gate (LLP 0293) walks every visible core command and refuses -Z, sparing only query sql. query grep is the second verb with a greedy positional, so it trips the gate. It joins the exemption rather than opting into strictness: grep's positional is search text, and a recorded transcript is mostly command lines, so -Z or --force is an ordinary thing to search for. Same bargain rg -- -Z strikes.

Verification

npm run typecheck: clean.
npm test: 5085 pass. Two failures, neither from this work: the graph help-text case that fails identically on a clean origin/master worktree on this machine, and claude-telemetry-unparseable-body (a concurrency test that passes 3/3 in isolation and is unrelated to search).
npm run smoke -- query_grep_roundtrip: ok. It drives the real CLI through scan tier, hyp purge --session, maintenance building sidecars, the coverage line, the indexed tier, and LLP 0105 withholding from three caller contexts.
Dogfood: hyp query grep hypgrep finds this feature's own development sessions in the local cache.

Note on coverage

The allowlist is nine columns. tool_args was dropped during #953's review: it is the dataset's one VARIANT column and no tier could ever produce a hit from it, so it cost brute-scan decode time while promising coverage it could not deliver. #977 restores it once hypgrep can index VARIANT.

🤖 Generated with Claude Code

…er file (LLP 0265 T4) src/core/search/grep_service.js is the client half of LLP 0264: the same two-tier search the server runs, folded onto the client's single cache. executeGrepSearch flushes the dataset's pending spool first (the query seam's own freshness move, now exported from sql.js), walks every live data file newest message-day first, and serves each file through its hypgrep sidecar (parquetFind, index proposes, shared matcher confirms) when one exists beside it, by brute scan under the narrow SCAN_COLUMNS projection when not. Budget, truncated/exhausted, hit shape and sort order mirror the server byte for byte through the shared core/search modules. Two row gates the server does not need: - Purge: a raw file read does not apply Iceberg position deletes, so the walk carries each file's committed delete positions (new listLiveDataFiles export on the iceberg store) and filters both tiers by them; a stale sidecar cannot resurrect a purged row (LLP 0104). - Visibility: every surfaced row passes the LLP 0105 lattice check via cwdWithheldFromCaller, hoisted out of withLocalOnlyVisibility so the SQL path and this walk share one predicate instead of two copies. The check runs after the match, so withheldRows counts hits the caller was not allowed to see and costs no result budget. The shared types gain GrepSearchParams, the wire shape every serving surface accepts. Tests cover both tiers end to end on a real Iceberg cache: locators and snippets, newest-first truncation, from/to and session/chain scoping, the JSON tool_args column matching through cellText, local-only withholding at every caller rank, purge on both tiers, the sub-ngram literal query answering exactly through an index, and spool-captured rows surfacing after the service's own flush. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… follows (LLP 0265 T6) Compaction is the moment a data file stops changing, so it is the one point where a hypgrep index can be built once and stay valid against its rows. maintainCache now follows every committed rewrite of the grep dataset with a sidecar-build pass over the new generation's files, in a worker thread (createIndex is seconds of straight-line CPU and the daemon is single-threaded), one file at a time. The worker handle and thread are ports of the server's index-worker pair, with one behavioral fix: the worker holds an event-loop ref exactly while a build is in flight, because an always-unref'd worker deadlocks any process whose loop would otherwise drain while awaiting the build. Sidecar existence is the completion marker, no ledger: the publish is a write-then-rename, a killed daemon leaves nothing half-claimed, and the next pass rebuilds whatever is missing. A file whose build keeps failing is quarantined after three attempts (in-memory, process-lifetime; a restart is the retry) and the scan tier serves it forever after: index presence is purely a performance property. The build pass can never fail the partition's own maintenance verdict. Two hazards found and closed on the way: - countDataFiles and measureDataDir counted sidecars (*.parquet in data/), which would have made every just-indexed partition read as "grew since compaction" and rewrite itself every tick through the LLP 0199 baseline gate. Both now exclude .index.parquet; a test pins that a second unforced tick stays converged. - A corrupt sidecar used to fail the whole search; the indexed tier now runs into local buffers and commits only on success, so an unreadable sidecar degrades that one file to the brute scan with no double count. GREP_DATASET joins the shared searchable-columns module so the search service and the build pass cannot disagree about which dataset carries indexes. Tests: per-file build and existence-marker idempotency, the quarantine budget with the scan tier still serving, the corrupt-sidecar fallback, maintenance building indexes for exactly the grep dataset, sidecars not re-triggering compaction, and a retired generation dying whole with its sidecars inside (the no-GC-code guarantee). The compaction-effectiveness tests' liveDataFiles helper learns the same sidecar exclusion the production counters did. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…sion-safe publish, and failures that name their file - Hoist `sidecarPathFor` beside `GREP_DATASET` in searchable_columns.js. The build pass and the search service each carried their own copy of the `.index.parquet` rule; two copies of a path contract drift into a build that writes an index nobody probes for. - Give the publish scratch file a random token. A fixed `<sidecar>.tmp` is only atomic for a single writer: the daemon tick and a hand-run `hyp` over the same cache would interleave into one scratch file and rename the mixture into place as a finished sidecar. The scratch file is now also removed on the failure path. - Name the data file on `grep_index.build_failed` / `grep_index.file_quarantined` / `grep_search.sidecar_unreadable`, and add the component/operation attributes, so three warnings can be told apart as one poisoned file or three. - Append rather than spread the indexed tier's buffered hits: `limit` reaches the service unvalidated and one file can fill the budget. - Correct the module docs: the pass runs only behind a committed compaction, which always publishes a fresh generation, so it never re-attempts a file it skipped or failed on, and a daemon restart is not a retry. Add the LLP 0264#lifecycle ref the module realizes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…failing open Review fixes on the T4 grep service (LLP 0265): - The limit returned the OLDEST matches. `hits.length = limit` ran before `sortHits`, so the cut fell in walk order; rows inside one data file run oldest to newest, so `--limit 3` over today's session answered the first three matches, not the newest three. Hits are now trimmed in sort order (amortized, so the buffer stays bounded), and the file walk stops only once every file still ahead is strictly older than the oldest kept hit. - `AbortSignal.timeout` threw out of the service. Its reason is a DOMException named `TimeoutError`, which the `AbortError` name check rejected, so the documented "partial answer, marked not exhausted" became an error for the deadline shape the signal exists to carry. - `chainId` without `sessionId` was silently discarded, answering across every session instead of the chain's. - `limit` is validated beside the query: an absent one made the budget NaN and walked the whole cache, a negative one threw a bare RangeError. - `listLiveDataFiles` swallowed a metadata load failure and answered `[]`, so a corrupt table made grep report zero hits where `hyp query sql` raises. It now propagates, matching `dataSourceForTable`. - `sortHits`' tiebreak returns 0 for equal keys, now that the buffer is sorted repeatedly. Five regression tests, each verified to fail on the pre-fix tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The sidecar existence probe only ruled out a missing index. A sidecar that exists but cannot be read (a half-written index from a killed build, a truncation from a full disk, a format the installed hypgrep refuses) throws from inside parquetFind, where the footer is parsed and the version checked, and nothing caught it: one poisoned sidecar failed every grep over the whole cache, including the partitions the walk never reached. That makes index state a correctness input, which LLP 0264 #lifecycle says it never is. The indexed read now degrades to the scan tier when the index proves unusable before it produced a row, and the reader catch no longer special-cases ENOENT: any unreadable sidecar is an unindexed file. A failure after the first row still propagates, because retrying it as a scan would count the collected hits twice. Also give the file-walk comparator a 0 for equal days, like sortHits: one day is many files and the early break reads the walk as strictly day-descending, so same-day order should come from the comparator rather than from whatever the engine's sort happens to do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…lowlist Merge of the base branch plus the maintainer's decision on #953. ## Conflict: src/core/search/grep_service.js Both branches independently fixed "a poisoned sidecar fails the whole query", so the conflict is two spellings of one fix, not two intents. - t4 (`b590b110`) hoisted a `searchIndexed` helper that collects through the shared `collect`/`trimHits` machinery (its `8b694e5b` sort-order truncation) and degrades ONLY when the read failed before the first row; a later failure propagates, because the rows already pushed to the shared buffer could not be taken back. - t6 buffered the attempt into a local array and committed on success, so any failure degrades, and logged `grep_search.sidecar_unreadable` naming the file that needs deleting. Taken: t4's structure with t6's buffering and log folded in. The helper keeps its name, the 2-argument `isAbort(err, signal)` t4 introduced (t6 still called the 1-argument form, which would have turned a timeout into a thrown error), the day-descending early break, and sort-order truncation; it gains the local buffer, so a sidecar that tears mid-read degrades that one file instead of failing the query, and it gains the warning. `trimHits` is generalized to `trimBuffer(list)` so the per-file buffer is trimmed by the same rule as the shared one: buffering must not trade the walk's memory bound away, and cutting the buffer in walk order would reintroduce the bug `8b694e5b` fixed. ## Decision on #953: tool_args leaves SEARCHABLE_COLUMNS `tool_args` is the dataset's one VARIANT column. The index worker only indexes STRING leaves in the allowlist and the server's row predicate gates on `typeof value === 'string'`, so the column has never produced a hit on any tier in either repository: T3's `cellText` coercion made the client's scan tier uniquely able to match it, on a premise ("the indexed tier reads the column's own text") that was false on both sides. Rather than add coverage neither repo has ever had, the column is dropped and the loss is recorded. Follow-up: #977. - `SEARCHABLE_COLUMNS` loses `'tool_args'`; `SCAN_COLUMNS` derives from it, so the brute scan stops decoding the column too. - The module comment now gives the VARIANT reason and points at #977, in the spirit of server LLP 0157 #identifier-columns. - `cellText` keeps its coercion (it is what keeps `rowTest`, `test` and `locate` answering identically on any cell shape, and #977 needs it in place), but its comment stops claiming the indexed tier reads the column's text. Its `@ref` gloss is corrected to match. - `toHit` and `GrepSearchMatcher` lose the same false claim. - Tests: the pinned allowlist drops the column, a new test pins its absence from both the allowlist and the scan projection, and the matcher tests exercise the coercion through a column that is actually searchable while pinning that `tool_args` no longer matches. - New in `search-grep-service.test.js`: a row matching only in `tool_args` returns zero hits from BOTH tiers, scan and indexed, with the tier counters proving each one really served the file. The invariant is tier agreement, not coverage. No LLP change: LLP 0264 never enumerates the columns (it points at this module and defers to server LLP 0157), so nothing it settled is contradicted. Amending server LLP 0157 and the now-wrong `grep_search` tool blurb is hypaware-server work, tracked separately. `sidecarPathFor` stays the single contract owner in `searchable_columns.js`; no re-export from `sidecar_build.js` is restored (#954 takes the import fix on its side). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… scratch is not data bytes The merge resolution's per-file buffer degraded one file instead of failing the query, but it also made a deadline throw away everything the index had already produced for the file it landed in. hypgrep checks the signal at every coalesced range boundary, so a deadline lands inside a file, and on a newest-first walk that is the newest file the caller most wants. Committed before the abort propagates: safe because an abort ends the walk, so the file is never rescanned and no row can be counted twice. grep_search.sidecar_unreadable named only the sidecar, but parquetFind opens the source data file through the same factory and runs the row filter per row, so a torn source parquet lands in that catch too and points the operator at a healthy index. Renamed to grep_search.indexed_read_failed and both files are named. measureDataDir excluded `*.index.parquet` but not the build's publish scratch, `<file>.index.parquet.<uuid>.tmp`, which survives a kill between write and rename with no reaper until the generation retires. countDataFiles already skips it, so counting its bytes broke the shared-file-set invariant in the dangerous direction: needsCompaction compacts on a LOW average, so a large orphan makes a fragmented partition read as healthy. Test pins it. Also recorded, not fixed: a sidecar freezes the allowlist it was built over (hypgrep stores hypgrep.text_columns in the index and prunes to it, and nothing compares that stamp to today's SEARCHABLE_COLUMNS), so #977 has to invalidate existing sidecars rather than only build new ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

queryGrepVerb in src/core/search/grep_verb.js joins CORE_VERBS beside query sql: one declaration projects the `hyp query grep` CLI command and the `grep_search` MCP tool, and because the tool name and inputSchema match the server's own grep_search (query, regex, session_id, chain_id, from, to, limit), `--remote <target>` reaches the server's archive-backed search with no server-side feature work. A server host displaces the kernel verb with its own via unregister (T2's affordance, server LLP 0178), so shipping it is safe on every host. The plan's open argv question resolves itself: the codec already maps `--session-id` onto the snake_case wire name, so no alias was needed. `include-local-only` is the one local-only parameter and deliberately carries NO schema default: argvToParams sends every defaulted property over the wire on --remote, the server's schema does not know the name, and a default would therefore fail validation on every remote call. A test pins the absence. The render flattens hits to one row per matched column (locators lead, snippet trails, part_id ready to pivot into query sql) and delegates to the shared query formatter, which is what gives grep the LLP 0225 contract for free: table/markdown escape captured bytes for a human reader, json/jsonl stay byte-exact for a pipeline, and the context budgets and --output spill behave exactly as query sql. Truncation, an uncovered walk, freshness, and local-only withholding each get their own stderr line; stdout stays a valid render. Tests: wire-schema pinning (property set, required, the no-default hazard, the coverage clause), codec flag mapping, end-to-end CLI runs over a real cache (hit render, newest-first, escape-vs-byte-exact, truncation notice, out-of-range limit fallback), a bare server-shaped result rendering without the local fields, and a stubbed remote MCP server proving the wire params travel exactly and the same render draws the server's answer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…he snippet last Review fixes on top of the T5 verb, each reproduced against a real Iceberg cache before the change: - `--limit 5000` answered with 50 rows (the default) and then advised "raise --limit", advice the caller had already followed and could not follow further. Above the ceiling now clamps to the ceiling the flag's own help advertises; only an unusable value (absent, fractional, zero) still falls back to the default. - `--from 2026-8-1` rendered an empty answer, exit 0, nothing on stderr: the window is compared lexicographically, so a mistyped day prunes every real one. A verb whose summary works this hard to make "zero hits" mean something must not let a typo forge one, so a day bound outside YYYY-MM-DD is refused with the flag named. - The snippet sat between the locators, and `renderTable` bounds a column's width but never truncates a cell, so any snippet past 80 characters pushed `message_id` and `part_id` out of column on exactly the rows a reader scans. Locators now lead and the snippet trails, as the render comment always claimed, with the row keys inserted in the same order so `--format json` and the table agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…s from lying (#952 review 2) Three edges the verb owned and got wrong: - `core_commands.js` projects every CORE_VERBS entry pre-boot so `hyp --help` can render, so the top-level `grep_service.js` import pulled hypgrep, hyparquet and the Iceberg store into the front door of every `hyp` invocation. Measured on `hyp --help`: 173-176ms with the eager import, 158-159ms with it deferred, 157ms on a tree with no grep verb at all. Now loaded inside `operation`, the way `verb_command.js` already defers the remote stack. - At the 1000-hit ceiling the truncation notice still said "raise --limit", which is the exact advice the clamp exists to avoid printing at a caller who cannot follow it. The operation now reports whether the ceiling was the binding limit and the notice names the ceiling instead. - Zero hits over zero searched files rendered identically to zero hits over the whole cache: empty stdout, empty stderr, exit 0. The summary spends 500 characters making "zero hits" honest for an MCP caller, and the un-searched case counterfeited one. The service already returns `indexedFiles`/`scannedFiles`; the render now says when both are zero. Quiet on `--remote`, which carries no file counts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…oundtrip smoke (LLP 0265 T7) The last task of the grep plan, on top of T5 (the verb) and T6 (the sidecar build): - hyp query status reports grep-index coverage: a summary line (grep index: N of M data files indexed) plus an indexed= extra on each searchable partition, computed by a pure directory scan in cacheStatus (indexedFileCount, grep dataset only). "Grep is slow on deep history" is now diagnosable where the operator already looks, and the expected gap (fresh files index only at compaction) is explained in the line itself. - The hypaware-query SKILL.md (both host copies) documents the grep subcommand: when to prefer it over LIKE-SQL, the ten-column coverage caveat and what zero hits does not prove, the truncation notice, the coverage-versus-speed relationship with the status line to check, the sub-ngram literal cliff (a short literal defeats index pruning but never correctness), and the local-only withholding parity with SQL. The read-class verb lists gain grep_search. The host-divergence fixture is re-recorded (the two deliberately host-specific lines both carry the verb list). - A hermetic smoke, query_grep_roundtrip, drives the real CLI through the whole story: scan-tier search before any index exists, hyp purge --session removing a row grep can then no longer surface, hyp query maintain building sidecars, the status coverage line, the indexed tier answering identically (proved from query.grep_search span attributes: indexed>0 scanned=0, and no query text in any span), and LLP 0105 withheld/visible/override from three caller contexts. Added to the release battery in AGENTS.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…he sidecar name Findings from the neutral review of #954, all low severity, no behavior change to the shipped surfaces: - The roundtrip smoke's "newest visible hit leads" check accepted either sess-purged or sess-new, so a one-day sort inversion passed it. The seed dates make the answer exact; pin it to sess-new. - The smoke's header claimed step 3 runs `hyp query maintain --force`, but the step calls `maintainCache` directly (it asserts on the sidecar counters in the returned report). Say what the code does. - The post-shutdown telemetry assertions were wrapped in `step()`, which opens a root span against an already-shut-down provider: the smoke_step never reached the trace. Unwrapped, matching every sibling flow. - `countIndexedDataFiles` restated the `<file>.index.parquet` pairing rule that `sidecarPathFor` already owns, in the one place a drift would silently misreport coverage. Import it instead. - `search-sidecar-build.test.js` claimed indexedFileCount "stays absent elsewhere" without a partition where it could be absent. Add one. - The hypaware-query skill pointed at `hyp query status` two paragraphs after routing cache operations to `hyp cache`. Both names work (alias); use the canonical one. The divergence fixture hashes host-only lines, so an identical edit to both copies leaves it unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…h completeness notices, and the two remote exceptions The doc is the surface an agent acts from, so a caveat it omits becomes a wrong call, not just a gap: - `--limit` was named as the remedy for truncation with no ceiling. The verb accepts any integer through its schema and then silently substitutes the default for anything outside 1-1000, so `--limit 5000` returns 50, fewer than the caller asked for and with no signal. Stated. - Only one of the verb's two stderr completeness notices was documented. `grep: the search stopped before covering every file` means the walk aborted, which a wider limit cannot fix; reading it as truncation sends a caller the wrong way. Both are now named and told apart. - `--remote` was described as running "the same search", but a server restricts `--regex` to its operator and rejects `--include-local-only` outright. Both exceptions stated beside the flag. Also aligns the new quick-reference line's trailing comment with its neighbours (it sat one column right). Both host copies carry identical edits; all four touched lines are shared, so the host-divergence fixture is unchanged.

…: nine columns, tool_args moves to the not-searched list The maintainer's unstick on PR #954 settled that #953 drops tool_args from SEARCHABLE_COLUMNS. The verb's coverage clause interpolates the constant so it follows on its own, and the query_grep_roundtrip smoke never seeds or asserts a tool_args hit, so the SKILL.md enumeration (both host copies, one shared line) was the only surface stating the old set. The two deliberate host-only lines are untouched, so the divergence fixture needs no re-record. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The T7 review commit that stopped restating the sidecar name imported sidecarPathFor from sidecar_build.js, which is where it lived when T7 was branched. #953's own review round moved it beside GREP_DATASET in searchable_columns.js, so re-stacking T7 on the current T6 tip left the import naming an export that is no longer there: a typecheck error and every maintenance test red. Point it at the module that exports it now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Master's D1 short-flag gate (LLP 0293) walks every visible core command and refuses an unknown `-Z`, sparing only `query sql`. `query grep` is the second verb to bind a greedy positional, so integrating the two branches trips that gate. It belongs in the exemption rather than opting into strictness. LLP 0293 settles the mechanism ("the verb family never opts in"), and grep's positional is search text, not a flag: a recorded transcript is mostly command lines, so `-Z` or `--force` is an ordinary thing to search FOR. Refusing it would make the one obvious way to find a flag in your own history exit 2, which is the bargain `rg` already strikes with `rg -- -Z`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Two findings from the review of 237b081: - Both `hypaware-query` SKILL.md copies told agents that `--limit 5000` is "silently replaced by the default rather than clamped, so it returns 50". The verb clamps: `Math.min(rawLimit, MAX_LIMIT)`, pinned by `test/core/query-grep-verb.test.js`. Only an unusable value (zero, negative, fractional) falls back to 50. An agent reading the old text would pick a limit it believed was 50 and silently over-fetch, or avoid the flag entirely. - `executeGrepSearch` built its settle list from the spool walk PLUS partition discovery, which overlap by construction (the spool dir sits inside the partition dir). `settlePendingCacheForQuery` is per-entry, so a pending-but-debounced table pushed its "last write to query cache was N minutes ago" staleness line once per copy: grep printed it twice where sql prints it once. Deduped by table path. Also corrects the indexed tier's comment, which claimed "the generator is simply not pulled past the budget" when the loop drains it. It has to: rows inside one file arrive in write order, so stopping at the budget would keep that file's oldest matches. The sort-order trim on `found` is what bounds the memory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`hyp cache status` reported `grep index: N of M data files indexed` with M taken from `dataFileCount`. icebird writes position deletes into the live `data/` directory as `<uuid>-deletes.parquet`, so that counter includes them, and no sidecar is ever built beside one. Any partition purged (or retention-trimmed) since its last compaction therefore reported permanently incomplete coverage, plus the parenthetical advising a compaction that cannot close the gap. `cacheStatus` now measures the denominator in the same directory scan that finds the sidecars and reports it as `indexableFileCount`; the CLI line reads that, falling back to `dataFileCount` for an older report shape. The smoke did not catch it because it checks coverage right after a compaction, which rewrites the deletes away into a fresh generation. The new test purges after the index build, which is the order a user hits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two review findings on the grep verb's surface. The "searched nothing" notice fired on an aborted walk. `executeGrepSearch` leaves `indexedFiles` and `scannedFiles` at zero when the deadline lands before the first file is served whole (the indexed tier deliberately does not count an interrupted file), so a caller whose search was cut short over a full cache was told "nothing is recorded on this machine yet". It is now gated on the walk having finished; with no hits the budget break cannot have fired, so `exhausted === false` is exactly the abort case the notice above already reported. The short-flag lenient set's rationale claimed more than the codec gives. The verb codec reads every `--` token as a flag and has no end-of-flags escape, so `hyp query grep --force` and `hyp query grep -- --force` both still exit 2, exactly as `hyp query sql` does for the same argv; the exemption buys the single-dash case only, and `rg -- -Z` has no equivalent here. The comment also quoted LLP 0293 saying "the verb family never opts in", which the decision does not say: it gives the verb family the lenient reading and has the core set opt in through `strictShortFlags`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

test/core/query-sql-budget.test.js's "exceeds the execution budget" case used maxHeapBytes: 1 with 6000 short string rows, expecting any retained growth to trip the guard. On Node 24 it flaked (~50% locally, reproduced with node --test in a loop): the confirm-with-gc step (LLP 0097#confirm-with-gc) forces a full GC before refusing, and the few hundred KB of genuine retained growth from that many short rows was frequently smaller than the ambient heapUsed drift the same forced GC produces from unrelated baseline noise, so raw/settled came out negative and the crossing was never confirmed. Pad each retained value so genuine growth is an order of magnitude above that noise floor. Verified 25/25 clean runs on Node 24.19 (vs. consistent flakes before), plus under CPU load and a constrained --max-old-space-size, with npm test green on both Node 22 and 24.

…on index line carries its denominator Two review findings on the grep integration: - docs/CLI_REFERENCE.md documents every visible command shipped with HypAware, and `hyp query grep` is a new visible core command that arrived without a section. The PR updated AGENTS.md and both hypaware-query SKILL.md files but not the reference, so the one page that claims completeness was the one page missing the command. - `hyp query status`'s per-partition line printed a bare `indexed=N` beside `files=M`, where `files=` is `dataFileCount` and counts the position-delete files sharing the data directory. No sidecar is ever built beside a delete file, so a partition purged since its last compaction read as permanently under-indexed - exactly the misreading the aggregate coverage line already avoids by using `indexableFileCount`. The extra now carries its own denominator.

…and a corrupt local-only list stops blaming a sidecar Three findings from the code-review pass on this head: - `parquetFind` was called with no `columns`, and it forwards its extra options straight into its own `parquetReadObjects`. So the indexed tier decoded EVERY column of every candidate range, `system_text` and `raw_frame` included, while a candidate range is a whole coalesced run of blocks capped only at row-group boundaries. On a compacted gateway file that can decode more bytes than the brute scan reads for the same file, which is the one cost the tier exists to remove. Both tiers now read under `SCAN_COLUMNS`; the two comments that asserted the old behavior are corrected rather than left to mislead the next reader. - The indexed tier's catch also swallowed `LocalOnlyListUnreadableError` thrown by the `withheld` predicate inside its row loop. A corrupt machine-local list therefore logged `grep_search.indexed_read_failed` against a sidecar that was fine (whose stated remedy is to delete it), re-read every candidate file from scratch on the scan tier, and only then raised the identical error. It now propagates, matching the SQL wrapper's fail-safe polarity (LLP 0080 #fail-safe). - `listLiveDataFiles` returned a `recordCount` no consumer reads and the doc comment does not claim. Dropped, so the new contract is honest.

philcunliffe · 2026-08-24T18:17:53Z

Review round 1 at `4e6299b`

Verdict: approve with five fixes applied (pushed as 981107c and 8613a63), six notes left open.

No defect found that makes a search return a wrong result. The two tiers, the purge gate, the LLP 0105 gate, truncation-in-sort-order, the abort semantics, and the worker lifecycle all hold up under reading, and the tests around them are unusually thorough for a feature this size. The one finding with real teeth is a performance hole that let the indexed tier decode more bytes than the brute scan it exists to beat; the rest are a documentation gap, a misdiagnosed error path, an operator-facing counter that reads wrong, and a dead field.

Checks in a clean worktree at the fixed head: npm test 5090 pass / 0 fail / 1 skipped, npm run typecheck clean, npm run smoke -- query_grep_roundtrip, status_diagnostics, and gateway_claude_capture all ok.

Fixed in this round

1. Medium, performance. The indexed tier decoded every column of every candidate range.
Evidence: src/core/search/grep_service.js:311-318 (pre-fix) called parquetFind with no columns. hypgrep forwards its extra options straight into its own read (node_modules/hypgrep/src/parquetFind.js:63-70), so each candidate range was decoded with the full schema, system_text and raw_frame included. A candidate range is a whole coalesced run of blocks capped only at row-group boundaries (parquetFind.js:48-56), so on a compacted gateway file the "fast" tier could decode more bytes than the brute scan reads for the same file. That is exactly the cost SCAN_COLUMNS was introduced to remove, and the 90.8%-of-decoded-text measurement src/core/search/searchable_columns.js:103 cites is the measurement of it.

Two comments asserted the old behavior as if it were intended (searchable_columns.js:105-107, "The indexed path is unaffected ... this list does not reach it"; and the module header at grep_service.js:30-31), so this was a stated fact rather than a considered trade-off.

Fixed: columns: SCAN_COLUMNS is passed down, and both comments are corrected. Safe because every reader downstream of the call (accept, the withheld predicate's cwd, and toHit's locators) names only columns inside the projection, and hyparquet ignores a projected name a file lacks, which the scan tier already relies on for received_at. The tier-equality test at test/core/search-grep-service.test.js:323 ("the two tiers answer identically") is the direct regression proof and still passes.

2. Low, wrong diagnosis plus a redundant full re-read. The indexed tier swallowed LocalOnlyListUnreadableError.
Evidence: the withheld predicate runs inside the for await at src/core/search/grep_service.js:321, inside the try whose catch at :328 degrades the file. src/core/query/visibility.js:82-84 and src/core/usage-policy/local_only.js:35 make a corrupt machine-local list throw, and both the SQL wrapper and cwdWithheldFromCaller deliberately let it propagate (LLP 0080 fail-safe polarity). Pre-fix, a corrupt list instead logged grep_search.indexed_read_failed naming a sidecar that is fine, whose documented remedy is to delete it, re-read every candidate file from scratch on the scan tier, and only there raised the identical error.
Fixed: if (err instanceof LocalOnlyListUnreadableError) throw err ahead of the degrade path.

3. Low, docs. docs/CLI_REFERENCE.md gained no hyp query grep section.
Evidence: src/core/cli/core_verbs.js:22 adds queryGrepVerb to CORE_VERBS, making hyp query grep a new visible core command, while docs/CLI_REFERENCE.md:111-160 documents query overview, query sql, query schema and every plugin query verb. The page opens with "This reference documents the visible commands shipped with HypAware", and the most recent comparable change (#969, hyp remote mint) updated it in the same PR. This PR did update AGENTS.md and both hypaware-query SKILL.md files, so the one page claiming completeness was the only surface left out.
Fixed: a ### hyp query grep section covering the flags, the nine-column coverage caveat, the index-versus-scan speed property, the local-only override, and the two --remote exceptions.

4. Low, operator-facing counter. The per-partition status line printed a bare indexed=N beside a files=M that counts delete files.
Evidence: src/core/commands/query.js:118 (pre-fix) sat on the same line as files=${p.dataFileCount} at :123. dataFileCount excludes sidecars but not -deletes.parquet (src/core/cache/maintenance.js:1518-1530), while indexedFileCount is measured against indexableFileCount, which excludes both (maintenance.js:1556-1572). test/core/search-sidecar-build.test.js:178-201 pins exactly that divergence for the aggregate line. So after any purge the per-partition line read files=3 indexed=2 on a fully indexed partition: the "permanently under-indexed, advise a compaction that cannot close the gap" misreading the aggregate line at query.js:96-106 was deliberately written to avoid.
Fixed: the extra now carries its own denominator, indexed=N/M.

5. Low, dead field in a new API.listLiveDataFiles returned recordCount (src/core/cache/iceberg/store.js:490, pre-fix) that no consumer reads and the doc comment does not claim. Dropped.

Open, needs a human

6. Medium, LLP hygiene. The shipped visibility and purge mechanism diverges from an Accepted decision, and nothing in llp/ records it.
LLP 0264 #visibility states the scan "wraps its per-partition source in the existing withLocalOnlyVisibility wrapper", and that "purged rows are handled below the wrapper already: the icebird source applies position deletes". LLP 0265 T4 repeats it. The implementation does neither: src/core/search/grep_service.js:172-186 walks raw data files through listLiveDataFiles, applies committed position deletes by hand (:296, :330), and applies the lattice through the newly exported cwdWithheldFromCaller (:142, src/core/query/visibility.js:72-92).

The change is well argued in the code comments and preserves the decision's intent (one shared predicate, the lattice not reimplemented, purge honored): a file-level two-tier walk genuinely cannot route through an AsyncDataSource. But CLAUDE.md's "land the doc edit in the same commit as the code" is not satisfied by comments, and 0264 is Accepted while 0265 is Active, so the right move is a new LLP that @refs what it changes plus an Extended-by: forward-ref on both, not an edit. All 21 commits on this branch touch zero files under llp/.

I did not mint that LLP. Picking a number against concurrent branches and authoring a Decision are both calls that should be a human's.

7. Low. Sidecars are only ever built behind a committed compaction, so a partition already at its file-count floor is never indexed, and the status line's advice is then unactionable.
src/core/cache/maintenance.js:257 gates the build on report.compacted, and src/core/search/sidecar_build.js:30-36 documents that there is deliberately no retry. A cache whose grep partitions were compacted before this feature shipped will sit at grep index: 0 of N indefinitely, while src/core/commands/query.js:105 tells the operator "(searches brute-scan the rest; compaction indexes them)": advice a needsCompaction-negative partition cannot follow. This follows LLP 0264 #lifecycle by design, so it is a product gap to decide on, not a defect to patch. Worth a follow-up issue if existing installs are meant to reach coverage.

8. Low. The sidecar build is unbudgeted work inside the budgeted maintenance loop, and reads whole data files on the main thread.
src/core/cache/maintenance.js:257-287 runs after the only budgetMs check (:180-184), and buildSidecarsForTable takes neither a deadline nor an abort signal (src/core/search/sidecar_build.js:103). Each iteration also does await fsPromises.readFile(sourcePath) of a whole compacted data file on the main thread before transferring it (sidecar_build.js:139); target_file_bytes defaults to 128 MB (maintenance.js:52), so the daemon holds a file-sized buffer resident per build. src/core/search/index_worker.js:8-11 puts the CPU cost at "seconds per sidecar". The worker keeps the CPU off the loop but neither the IO nor the allocation, and LLP 0199's neediest-first budget does not cover the new work. Bounded in practice (the cutoff fires on the next iteration). Flagged rather than patched, since adding a deadline changes the pass's contract.

9. Low. --from and --to are shape-checked but never checked against each other.
src/core/search/grep_verb.js:169-175 accepts --from 2026-09-01 --to 2026-08-01, which then prunes every file. Worth noting the failure is milder than it first looks: with every file pruned, indexedFiles and scannedFiles are both zero, so the render's notice at grep_verb.js:150-152 does fire and does name --from/--to. The genuinely silent case is narrow (a file whose partition day will not decode is never pruned, so it is searched and the notice is suppressed). Still, an inverted window is never meaningful, and dayBound's own docstring says a mistyped flag must not be able to forge a zero. Left open because refusing it is a CLI behavior change that wants its own test.

10. Low. A malformed --from/--to exits 1, not 2, unlike every other argument-validation failure in the verb family.
dayBound throws from inside operation, and src/core/cli/verb_command.js:129-134 turns that into exit 1 with no usage line, while argvToParams failures exit 2 with usage (verb_command.js:91-96). The check itself is right and load-bearing. The exit code is the codec's shape: VerbInputProperty has no pattern (hypaware-plugin-kernel-types.d.ts:1657-1666), so moving the check into the schema is a codec change, not a line in the verb. Noting so it is a decision rather than an accident.

11. Info, release coordination. LLP 0264 #verb calls shipping the kernel verb without the server-side displacement "a regression on every server host". The kernel half is present (verbs.unregister at src/core/registry/verbs.js:55, retraction at :145-182) and src/core/cli/core_verbs.js:16-18 documents the contract. Confirm hypaware-server #364 is merged before any release carrying this, and before hypaware-server bumps its hypaware dependency.

What I checked and found sound

Truncation order.trimBuffer/sortHits cut in sort order, not walk order, on both the shared buffer and the indexed tier's per-file buffer; the day-descending early break is correct because trimHits caps at budget first, and truncated is guaranteed true on that path. sortHits returns 0 for equal keys, which matters given the repeated sorts.
Purge. Position deletes are applied on both tiers by row position, and listLiveDataFiles propagates a metadata load failure rather than answering zero, matching the SQL path's polarity. Pinned by test/core/search-grep-service.test.js:305, :342.
Visibility.cwdWithheldFromCaller is now the single predicate shared with withLocalOnlyVisibility (visibility.js:182), the check runs after the match so withheldRows is honest and an out-of-rank row consumes no budget, and suppressedRows: 0 is correct because ai_gateway_messages declares no localOnlyContentColumns (only context-graph does).
Case-insensitivity across tiers. hypgrep lowercases both index and query n-grams (ngrams.js:42), and rowFilter: accept overrides hypgrep's own default match, so the indexed tier cannot prune away a differently-cased hit the scan tier would find.
Abort.isAbort compares against signal.reason identity, so AbortSignal.timeout's TimeoutError DOMException is handled, and the indexed tier commits its buffer on abort precisely because an abort ends the walk (no double count on rescan).
Worker lifecycle. Per-spawn owned map, ref-while-pending, failAll on both error and exit, the zero-length-index protocol guard, and the pooled-Buffer copy before transfer are all right.
Counter isolation.countDataFiles and measureDataDir both exclude sidecars and the publish scratch, so a just-indexed partition does not read as "grew since compaction" and get rewritten every tick; pinned by test/core/search-sidecar-build.test.js:203, :214. Sidecar GC is pinned at :254.
Conventions. No semicolons, no em dashes anywhere in the diff, JSDoc types only, @import at file top with repo-root-anchored .js specifiers, and every @ref anchor resolves (LLP 0264#decision|#shared|#verb|#visibility|#lifecycle, LLP 0265#out-of-scope, LLP 0105#override).

philcunliffe · 2026-08-24T18:52:27Z

neutral: rounds +2 — granted by Slack user U099BSGPZU4 via Slack

Requested in the #984 thread at 18:52Z, while review round 2 (dispatched 18:41Z at head 8613a637) was still running. The repo cap is the CLI default of 2 (.neutral/config.json sets no maxReviewRounds), so this raises the budget to 4 rounds and keeps the fix loop going instead of triaging residual findings to a follow-up issue at the end of round 2.

Recorded by the mayor on the human's ask; I did not decide this.

…t the build, and refuse a window that selects nothing Four fixes from the round-2 review of #984, plus the LLP that records what the shipped mechanism does differently from LLP 0264/0265. The sidecar build ran only behind a committed compaction, so a partition already at the compaction floor never rewrote, never indexed, and every grep brute-scanned it for the life of its generation, while `hyp cache status` advised a compaction that would not run. Coverage is the gate now (one readdir of the live data directory), which is sound because what makes an index safe is that a committed data file never changes its rows, not that a compactor wrote it; compaction is where indexing is cheapest, not what makes it correct. The pass is bounded by the maintenance tick's own deadline and reports `sidecarsDeferred`, because indexing is seconds of CPU per file and an unbudgeted pass appended after the cutoff undoes what the budget is for. It resumes on the next tick, which sidecar existence as the completion marker already made free. The first missing file of a pass is always attempted, for the reason the walk always works one partition. `--from` and `--to` are now checked against each other, and both day checks refuse with exit 2 instead of 1. An inverted window is two well-formed days that select nothing, so it rendered as a silent empty answer: the forged "nothing is recorded on this machine" the coverage clause exists to prevent. Exit 2 is the usage code, and a script that retries on 1 and reports on 2 must be able to tell a typo from a busy cache. The mechanism is `VerbUsageError`, available to any verb whose argument rule outgrows what `inputSchema` can state. LLP 0302 records the three places the shipped grep differs from the Accepted LLP 0264 and the Active LLP 0265 (the visibility lattice as a shared predicate rather than the source wrapper, purge applied by the walk from committed delete positions, and the build site and gate), plus the usage-exit rule. Both docs gain `Extended-by:` forward-refs to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

philcunliffe · 2026-08-24T19:01:29Z

neutral review round 2 (final) - head `8613a637`

Verdict: findings, six fixed and pushed. New head 8f9b3704 (two commits). npm test (5095 pass / 0 fail) and npm run typecheck are green at the new head, and the full release smoke battery from CLAUDE.md is green, query_grep_roundtrip included.

The search stack itself holds up under a second reading. The two tiers, the sort-order truncation, the abort semantics, the per-file degradation of a poisoned sidecar, and the LLP 0105 gate are all correct and unusually well argued in comments. Everything below is either a gap round 1 named and left open, or a doc line that fell out of fixing one.

The six round-1 open findings

#	Round-1 finding	Status at `8613a637`	Now
6	LLP hygiene: shipped mechanism diverges from Accepted LLP 0264 and Active LLP 0265; 21 commits touch zero `llp/` files	still open, confirmed	fixed - new `llp/0302`, forward-refs added to 0264 and 0265
7	Sidecars build only behind a committed compaction; a partition at the floor never indexes	still open, confirmed	fixed
8	The build is unbudgeted work after the `budgetMs` cutoff	still open, confirmed	fixed (budget); the whole-file read stays, see below
9	`--from`/`--to` not validated against each other	still open, confirmed	fixed
10	A malformed day exits 1 rather than 2	still open, confirmed	fixed
11	Release coordination with hypaware-server #364	materially resolved - #364 is MERGED	narrowed, see below

6. LLP hygiene - fixed

Three real divergences, re-derived against the current tree, not taken from the round-1 summary:

llp/0264 #visibility says the scan "wraps its per-partition source in the existing withLocalOnlyVisibility wrapper". It cannot: that wrapper decorates an AsyncDataSource, and the grep walk chooses its tier per file, so it reads data files directly and has no source to decorate. The shipped code hoists the lattice out of the wrapper into cwdWithheldFromCaller (src/core/query/visibility.js:92) and both surfaces call it.
Same section says purged rows "are handled below the wrapper already: the icebird source applies position deletes". Again there is no source: a raw parquet read applies nothing. listLiveDataFiles (src/core/cache/iceberg/store.js:473) hands the walk the committed delete positions and both tiers filter on them.
llp/0265 T6 says "compactGeneration queues an index build for each finalized data file". It does not; the pass hangs off maintainCache's partition loop.

Accepted/Active docs are immutable, so these are recorded in a new doc, llp/0302-grep-search-integration-divergences.decision.md, with Extended-by: forward-refs appended to LLP 0264 (header, #visibility, #lifecycle) and LLP 0265 (header, T6). Numbering: 0302 is the next free number across master and every remote branch (0300 is taken on win32-groundwork, 0301 on codex/bounded-compaction-resettle). Authorship is neutral, matching LLP 0293.

7. A partition at the compaction floor never indexed - fixed (was the real bug of the six)

src/core/cache/maintenance.js:259 gated the build on report.compacted. A partition that is not due for a rewrite (LLP 0199's baseline gate, or LLP 0217's ineffectiveness verdict) never compacts, so it never got a sidecar, every grep brute-scanned it for the life of its generation, and hyp cache status printed compaction indexes them - advice naming a rewrite that will never run.

The premise was also stronger than it needed to be. What makes an index safe is that a committed data file never changes its rows, which is true of every file in the table, not only of one a compactor just wrote. Compaction is where indexing is cheapest, not what makes it correct.

The gate is missing coverage now, measured by the countIndexCoverage readdir already in the file - the same cost profile as the counters beside it, and it reads zero-work whenever coverage is complete, so a fully indexed partition costs one directory read and no pass at all.

8. Unbudgeted build - fixed for the budget; the whole-file read is unchanged and defensible

buildSidecarsForTable now takes deadlineMs and stops between files once the tick's deadline passes, reporting sidecarsDeferred. It resumes on the next tick, which sidecar existence as the completion marker already made free - and which only became a real remedy once finding 7's gate stopped requiring a fresh compaction. The first missing file of a pass is always attempted, for the same reason maintainCache always works one partition: otherwise a busy cache would never index anything.

The fsPromises.readFile of a whole data file (sidecar_build.js:139) is not changed, and I do not think it should be. It is async I/O on the threadpool, not CPU on the loop (the createIndex seconds are already on a worker thread), so the cost is a transient buffer, and the pass is explicitly one file at a time to bound exactly that. Streaming it would mean streaming into hypgrep, which reads through an AsyncBuffer over a resident file. Recording as accepted by design, not a residual.

Side effect worth flagging to triage: the per-file poison bound (MAX_INDEX_ATTEMPTS) was inert under the old gate, because a failed file was never offered to a second pass. It now bites as designed. The module doc claimed the opposite ("What that does NOT buy: a retry") and has been corrected.

9 and 10. `--from`/`--to` - both fixed

dayBound validated shape only, so --from 2026-08-20 --to 2026-08-01 was two well-formed days selecting no day at all: every file pruned, empty answer, nothing on stderr. That is exactly the forged "nothing is recorded on this machine" that the verb's own coverage clause and zero-files notice exist to prevent, reached through the one door they do not watch. Now refused.

Both day refusals exit 2 instead of 1. runVerbCommand mapped every operation throw to 1, so a typo read to a script as "the search failed" - and a script that retries on 1 and reports on 2 answers a typo with a retry loop that can never succeed. The mechanism is a new VerbUsageError (src/core/cli/verb_errors.js): an operation throws it for a caller's argument mistake and a plain Error for everything else, and the wrapper prints the usage line beside it exactly as it does for a codec refusal. Available to any verb whose argument rule outgrows what inputSchema can state; LLP 0302 #usage-exit records why it is not a schema pattern (a cross-field rule has no schema form, and the wire schema is deliberately kept matching the server's grep_search).

Verified end to end, not just in tests:

$ hyp query grep needle --from 2026-8-1 -> exit 2 + usage line
$ hyp query grep needle --from 2026-08-20 --to 2026-08-01 -> exit 2 + usage line
$ hyp query grep needle --from 2026-08-01 --to 2026-08-20 -> exit 0

11. Server coordination - narrowed, and no longer a gate

hypaware-server #364 is MERGED ("The server's grep_search outranks a kernel-shipped twin", server LLP 0178). The sequencing risk LLP 0264 #open named, a server host ending up with two tools of one name, is spent. What remains is the server's import swap onto this repo's shared allowlist module: until it lands, two repositories hold two copies of one constant, and LLP 0264 #shared's "no tier can surface a match another tier cannot" rests on them being kept equal by hand. That is a follow-up, not a release gate. Recorded in LLP 0302 #residuals.

New findings this round

N1 (minor, fixed).docs/CLI_REFERENCE.md documented the exit-code bug rather than the contract: "A malformed --from or --to ... return 1". Rewritten alongside the fix, and it now names the inverted-window case too.

N2 (nit, fixed).docs/CLI_REFERENCE.md and both hypaware-query/SKILL.md copies said "Compacted files are served through sidecar indexes", which stopped being the mechanism with finding 7. Reworded identically in the claude and codex trees, so the skill-host-divergence line-set hash is unchanged.

Residuals for the triage rung

Nothing here blocks. My read of each:

received_at in SCAN_COLUMNS (src/core/search/searchable_columns.js:132). Justified as "the tier exclusion, the rule that keeps a row held by two tiers at once from being counted twice" - but grep_service.js:37 states plainly that the client has no cross-tier exclusion, and the same file notes the client's ai_gateway_messages carries no received_at at all. The column is harmless (hyparquet ignores a projected name the file lacks); the justification is the server's reason pasted into the client. Preference, a comment fix.
scannedFiles counts an interrupted file, indexedFiles deliberately does not (grep_service.js:418 increments before the read; grep_service.js:386 increments only after). An abort mid-scan therefore counts asymmetrically. Not user-visible: the zero-files notice that reads these is already guarded on exhausted !== false. Preference.
An invalid --regex pattern, and an over-length query, still exit 1. These are caller argument mistakes like the day flags, but they are refused inside matcher.js, which LLP 0264 #shared makes a module the server imports too - so it must not import a CLI error class. Left deliberately, and CLI_REFERENCE states the 1 honestly. Preference, and fixing it properly means a shared error kind, not a one-line change.
Catastrophic regex backtracking is unbounded locally.MAX_QUERY_LENGTH caps the pattern and the comment is explicit that this does not make regex mode safe (V8 cannot interrupt a running regex). Local-only and self-inflicted; a server restricts --regex to the operator. Accepted by design.
A data file dereferenced without a rewrite. A purge that empties a file can leave it on disk while the manifest stops listing it. countIndexCoverage reads the directory and keeps counting it as indexable, so the new build gate fires each tick and the pass finds nothing to do (one metadata load, zero builds) until the generation retires. Bounded and cheap; recorded in LLP 0302 #residuals rather than fixed.
The T6 test LLP 0265 promised is still half-owed. "Orphan sweep and retention delete sidecars with their files": retirement is pinned (a retired generation dies whole, sidecars included), retention is not. Retention reclaims whole partition directories, so a sidecar cannot outlive its file there either - nothing asserts it. Preference.

What landed

8613a637..e1f0294b, one commit:

File	Change
`src/core/cache/maintenance.js`	build gate is missing coverage, not `report.compacted`; passes the tick deadline down
`src/core/search/sidecar_build.js`	`deadlineMs`, `deferred` counter, corrected lifecycle and quarantine docs
`src/core/cli/verb_errors.js`	new `VerbUsageError`
`src/core/cli/verb_command.js`	maps it to exit 2 with the usage line
`src/core/search/grep_verb.js`	inverted-window check; both day refusals become usage errors
`src/core/cache/types.d.ts`	`sidecarsDeferred`
`src/core/commands/query.js`	status advice names maintenance, not compaction
`src/core/query/visibility.js`, `src/core/cache/iceberg/store.js`	`@ref`s to LLP 0302
`llp/0302-...decision.md`	new
`llp/0264-...`, `llp/0265-...`	`Extended-by:` forward-refs only
`docs/CLI_REFERENCE.md`, both `hypaware-query/SKILL.md`	N1, N2
`test/core/search-sidecar-build.test.js`	at-floor partition is indexed; a spent budget defers and a later tick finishes
`test/core/query-grep-verb.test.js`	exit 2 for both day refusals; `from == to` is a valid one-day window

Second pass: the `code-review` skill (run in an isolated worktree)

Round 1 noted that the direct-diff pass and the skill pass surface different bugs, so both were run. The skill pass landed after the direct-diff findings above were already fixed; it agreed with none of them and found two more of the same shape, both now fixed in a second commit, plus four it is right to leave for triage.

S1 (major, fixed) - the brute scan materialized a whole data file

src/core/search/grep_service.js:420 read the file in one call: await parquetReadObjects({ file: sourceFile, columns: SCAN_COLUMNS }). target_file_bytes defaults to 128 MiB (src/core/cache/maintenance.js:52) and the projection's bulk column is content_text, so a mature compacted cache decoded hundreds of MB of JS strings and row objects before a single row was tested. Two consequences, both real:

hyp query grep could exhaust the heap where hyp query sql over the same partition does not. The SQL seam streams for exactly this reason: scanRowsFromTable (src/core/cache/iceberg/store.js:536) exists so callers "never materialize the full table in memory".
The signal?.throwIfAborted() checks at grep_service.js:423 could not fire during the decode, so the deadline did not bound the step that dominates the wall clock.

It also made the module docstring's "the request's memory bound is one data file plus its index" a claim about the wrong quantity at the scale compaction actually produces.

Fixed by reading one row group at a time. The row group is the unit rather than an arbitrary row count on purpose: without the offset index hyparquet fetches and decodes a whole column chunk to serve any row inside it, so a fixed-size split would re-decode the same chunk once per slice and cost more than it saved. Group-aligned slices read each chunk exactly once, so the total decode is unchanged, only the peak drops, and a single-row-group file reads exactly as it did. Delete positions are file-absolute, so the group offset rides the lookup; a group-relative index would have resurrected purged rows in every group after the first. Docstring corrected to match.

S2 (minor by the reviewer's grading, fixed - I read it as the same class as findings 9/10) - an unusable `--limit` was answered, not refused

limit was declared { type: 'number' } with no minimum (grep_verb.js:71) and the operation then rewrote anything unusable to DEFAULT_LIMIT (grep_verb.js:92-95). So --limit 0, --limit -5 and --limit 2.5 each returned 50 hits and exit 0: a request for FEWER rows answered with more of them. That is the same forged answer dayBound goes out of its way to refuse three functions away in the same file, and the schema is also what an MCP caller validates against, so limit: 0 over the wire silently got 50. executeGrepSearch's own limit guard (grep_service.js:115) was unreachable from both surfaces as a result.

Declared { type: 'integer', minimum: 1, default: DEFAULT_LIMIT }, which coerceValue already turns into a named refusal. The ceiling deliberately stays out of the schema: above it the flag clamps rather than refuses, because the help text promises a capped answer and "raise --limit" is advice a caller already at the ceiling cannot follow. Docs and both skill copies updated (identically, so the divergence hash is unchanged).

$ hyp query grep needle --limit 0 -> exit 2 --limit expects a positive integer (got 0)
$ hyp query grep needle --limit -5 -> exit 2 --limit expects a positive integer (got -5)
$ hyp query grep needle --limit 2.5 -> exit 2 --limit expects a positive integer (got 2.5)
$ hyp query grep needle --limit 9999 -> exit 0 (still clamps to 1000)

S3 (major, left for triage - the one I would look at first) - unrestricted `--regex` on the local path

grep_verb.js:62-65 ships regex mode ungated, and its own help text says so: "Servers restrict regex mode to the operator; local search does not". matcher.js:16-23 is equally explicit that the length cap "deliberately does NOT claim to make regex mode safe from catastrophic backtracking, which V8 cannot interrupt". The compiled pattern runs per cell over every scanned row, so (a+)+$ wedges the thread with no abort path, and grep_search is registered on every host (core_verbs.js:22).

Why I did not fix it: the severity turns entirely on whether a client host's grep_search is reachable by anyone but the person at the terminal. If it is only ever the caller's own CLI process, this is self-inflicted and the server's operator-only gate is the right asymmetry. If a client daemon serves read-class verbs to a query-scoped remote caller, the same pattern takes ingest and the health probe with it, and that is a genuine availability hole. Deciding which, and picking between gating regex by auth class and moving the match into the killable worker-thread seam that already exists for index builds, is a design call with an LLP attached, not a review-round patch. Triage should treat this as the round's one candidate blocker.

S4 (minor, left for triage) - `exhausted` collapses two independent facts

grep_service.js:488 returns exhausted: exhausted && !truncated, and grep_verb.js:147-153 renders if (truncated) ... else if (exhausted === false) ..., so truncation always wins. A search that both fills the limit and aborts mid-walk prints only "more matches exist beyond the limit ... or raise --limit" - advice that cannot recover the files the walk never reached - while the "stopped before covering every file" line is suppressed. The shipped skill doc teaches those two notices as meaning different things. An MCP caller has the same problem: exhausted: false no longer separates "cut by the limit" from "never finished". Fixing it properly means keeping walk-completion distinct from truncation in GrepSearchResult, which is the shape the server mirrors, so it wants the cross-repo look this round cannot give it. Preference, but the most substantive one.

S5 (minor, left for triage) - a full-tree walk prepended to every grep

grep_service.js:181 calls discoverSpoolTables(storage.cacheRoot), which recursively readdirs the whole datasets/ subtree including every generation's data/ directory, on every grep. Its two other callers are background paths (the flush sweep, spool.js:188, and storage bootstrap, storage.js:502), not per-query. The SQL seam reaches the same spool tables in O(1) from the dataset's known label paths (ai-gateway/src/dataset.js:80-89, which always lists proxy_messages_v5 plus the v4 legacy label). Correct, just paid per query for information the dataset already knows. Preference.

S6 (nit, left for triage) - the index worker matches leaf names, not paths

index_worker_thread.js:97 pushes element.name, the schema leaf's own name rather than its dotted path. Correct for today's flat ai_gateway_messages, but a struct carrying a leaf named model or cwd would match SEARCHABLE_COLUMNS and hand hypgrep a textColumns entry naming something other than the top-level column the read side tests: a silent tier disagreement of exactly the kind this module exists to prevent. Unreachable on the current schema. Preference.

S7 (nit, left for triage) - `fs.existsSync` per data file on the query path

grep_service.js:406 probes each sidecar synchronously inside an otherwise fully async walk. On a large cache that is a sync stat storm on the daemon's loop. Note the surrounding catch already treats an unopenable sidecar as "no sidecar", so the probe could simply go away. Preference.

Checked and found sound (recording so it is not re-litigated)

The newest-first walk and its early break (file.day < hits[last].date is safe given day-descending order, ''-dated hits never trigger it, same-day files are still read); budget = limit + 1 and sort-order truncation; position-delete filtering on both tiers, including that parquetFind really does yield __index__ = range.rowStart + i (verified against hypgrep@0.5.1/src/parquetFind.js:79), so the BigInt(row.__index__) lookup cannot be starved by the columns projection; parquetFind staying serial because no limit is passed down, which is what actually holds the per-file memory bound; findDataFileEntries returning only non-status-2 data entries, so no delete file reaches the sidecar builder; hyparquet silently dropping projected columns a file lacks; the transferable() pooled-Buffer guard; cwd living inside SEARCHABLE_COLUMNS so the visibility predicate is never blinded by the projection; isAbort identity-checking signal.reason (so AbortSignal.timeout's TimeoutError is handled); and countDataFiles / measureDataDir / countIndexCoverage agreeing on one file set including the .tmp scratch name.

Final state

8613a637..8f9b3704, two commits. npm test 5095 pass / 0 fail, npm run typecheck clean, and the release smoke battery green including query_grep_roundtrip, re-run after the second commit.

Six fixed: the compaction-only index gate, the unbudgeted build, --from/--to cross-validation, the exit code for both day refusals, the whole-file scan read, and the unusable --limit. Plus LLP 0302 with forward-refs on 0264 and 0265, and the doc lines that fell out of those fixes.

Residual for triage, in the order I would rank them: S3 (unrestricted local regex - the only candidate blocker, and only if a client host's grep_search is reachable off-box), then S4 (exhausted collapsing two facts), then S5, then the preferences: S1's sibling items, S6, S7, received_at's justification, the asymmetric scannedFiles counter, exit 1 for an invalid regex, and the retention half of LLP 0265 T6's promised test.

… unusable --limit is refused instead of answered Two findings from the parallel code-review pass, both the same shape as the ones already fixed this round: work that is not bounded, and an answer that is not the one asked for. The scan tier materialized a whole data file. A compacted file runs to `target_file_bytes` (128 MiB by default) and the projection's bulk column is `content_text`, so it decoded hundreds of MB of JS strings before testing a single row: `hyp query grep` could exhaust the heap where `hyp query sql` over the same partition does not, because the SQL seam streams (`scanRowsFromTable`). The abort checks could not fire during that decode either, so the deadline did not bound the step that dominates the wall clock. It reads one ROW GROUP at a time now. The row group is the unit rather than a fixed row count because without the offset index hyparquet fetches and decodes a whole column chunk to serve any row inside it, so an arbitrary split would re-decode the same chunk once per slice and cost more than it saved. Group-aligned slices read each chunk exactly once: the total decode is unchanged, only the peak drops, and a single-row-group file reads exactly as it did. Delete positions are file-absolute, so the group offset rides the lookup. `limit` was a bare `number` in the schema and the operation rewrote anything unusable to the default, so `--limit 0`, `--limit -5` and `--limit 2.5` each returned 50 rows and exit 0. That is a request for FEWER rows answered with more of them, and it is the same forged answer `dayBound` goes out of its way to refuse. It is also the schema an MCP caller validates against, so `limit: 0` over the wire silently got 50. Declared `{ type: 'integer', minimum: 1 }`, which the codec already turns into a named refusal; the ceiling stays out of the schema because above it the flag clamps by design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…hare of the tick, and two completeness facts stop being one Closes most of LLP 0302 #residuals, plus four findings new to this head. New this round: - The brute scan's "one row group at a time" bound only covered decoded rows. Both tiers opened the source data file through the cache's `resolver.reader`, which is `readFileSync` of the whole file, so a 128 MiB data file stayed resident behind a walk that read it a row group at a time, and the read blocked the loop. Both tiers now use hyparquet's `asyncBufferFromFile`, so only the projection's own byte ranges are ever read (LLP 0303 #memory-bound). - The sidecar build inherited the maintenance tick's whole remaining tail. The walk is neediest-first, so the busiest grep partition goes first, meets a freshly compacted generation with zero coverage, and could starve every partition behind it of snapshot expiry and compaction on every tick. Capped at a share of the tick per partition (LLP 0303 #build-share). - A quarantined file kept coverage permanently short, so the pass paid a metadata load per tick for the life of the generation to rediscover there was nothing to do, and `sidecarsFailed` folded "skipped" into "failed". Directory-level short-circuit, and the two counts split. - An abandoned publish scratch (`<sidecar>.<uuid>.tmp`) leaked on a hard kill, was billed by no counter, and nothing ever removed it. The pass sweeps them past a grace window (LLP 0303 #scratch-sweep). Round-2 residuals closed: - `exhausted: exhausted && !truncated` collapsed two facts the skill doc teaches as separate, and the renderer chose between their notices with an `else if`, so a search that both filled its limit and aborted said only "raise --limit". Each fact is reported on its own, and the day-descending break no longer clears `exhausted`: it stops the walk having proved the remainder cannot change the answer (LLP 0303 #completeness-signals). - An invalid `--regex` and an over-length query exited 1. The shared matcher raises a refusal kind (`GrepQueryError`) and the verb maps it to exit 2, so the shared module still imports nothing from the CLI (LLP 0303 #query-refusal-exit). - The index worker matched schema leaves by name rather than by path, so a nested field called `model` or `cwd` would have handed hypgrep a column the read side never projects. Depth-one leaves only. - `discoverSpoolTables` walked every dataset's `data/` directory on every grep; scoped to the one dataset grep covers. - `fs.existsSync` per data file on the query path is gone: the reader's own ENOENT takes the same degrade path. - `scannedFiles` counted an interrupted file where `indexedFiles` deliberately does not; both now count a file served whole. - `received_at`'s justification described the server's tier exclusion as if it were the client's. - LLP 0265 T6's promised retention test: both retention paths, the row-level purge that leaves file and sidecar in place, and the directory reclaim that takes both. Left by decision: unrestricted local `--regex`. The condition its severity turned on is answerable from the tree, and the answer is no: the kernel's only MCP transport is stdio, which is the caller's own trust, and `--http` is refused outright. Recorded with the tripwire that flips it (LLP 0303 #regex-reachability). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

philcunliffe · 2026-08-24T20:17:27Z

neutral review round 3 - head `8f9b3704`

Verdict: findings, eleven fixed and pushed. New head 9ffd056b, one commit. npm test 5102 pass / 0 fail / 1 skipped and npm run typecheck are clean at the new head; query_grep_roundtrip, status_diagnostics, gateway_claude_capture, local_parquet_export, hypignore_capture_drop, cli_bundled_plugins_activated and daemon_foreground_start_stop are all ok.

Round 3 used its extra budget to close residuals rather than list them. Nine of the ten round-2 residuals are fixed; the tenth (S3) is answered rather than deferred, with evidence from the tree. Four findings are new to this head, one of them medium and one of them a claim the round-2 fix made but did not deliver.

New findings this round

N1 (medium, fixed). The build pass inherits the tick's tail and can starve every partition behind it

src/core/cache/maintenance.js:289 (pre-fix) passed deadlineMs: startMs + budgetMs: the tick's own deadline. That bounds the tick but not the walk. Partitions are visited neediest-first (LLP 0199 #neediest-first), so the busiest ai_gateway_messages partition is visited first, and a rewrite that just committed mints a new generation directory whose coverage is 0 of N. The pass then indexes at seconds of CPU per file until the tick is spent, the loop breaks at partition 2, and logs, traces, metrics and every other gateway partition get no snapshot expiry and no compaction, that tick and every tick after, because the busy partition keeps taking writes.

Nothing else in the loop had that shape: compaction is gated on a due verdict, so a healthy partition costs nearly nothing. The build was the first per-partition work whose cost is unrelated to whether the partition needs anything.

Fixed with GREP_INDEX_TICK_SHARE = 0.25: a share of the tick per partition, never its tail. Coverage still advances on a busy cache, because the pass always attempts its first missing file, which is the same guarantee maintainCache gives itself by always working one partition. Recorded as LLP 0303 #build-share.

Credit: found by the code-review skill pass, not by the direct-diff pass.

N2 (medium, fixed). The round-2 memory-bound fix bounded the decoded rows and not the bytes

Round 2's S1 changed the brute scan to read one row group at a time, and the module docstring then claimed "the request's memory bound is one row group ... never a whole 128 MiB data file". It was not.

Both tiers opened the source data file through io.reader, and the cache's resolver is fs.readFileSync of the whole file (src/core/cache/iceberg/resolver.js:26-29). So every slice the row-group split asks for comes out of a buffer that already holds the entire file: the raw bytes stayed resident however finely the decode was cut, and the read itself was synchronous on the loop for the duration. The row-group split is still worth having (decoded JS strings dwarf the bytes), but the stated bound was false and the loop-blocking half of the original finding was untouched.

Fixed: both tiers open the source through hyparquet's asyncBufferFromFile, which fetches per slice. The projection's own byte ranges are then all that is ever read - strictly less IO than reading the whole file, since SCAN_COLUMNS is nine columns of a much wider schema - and none of it is synchronous. The sidecar deliberately stays on the resident reader: it is the pruning structure, read in many small random ranges by queryIndex, and a fraction of the size of the file it indexes. LLP 0303 #memory-bound.

N3 (low, fixed). A quarantined file bought a metadata load every tick, forever, and reported a fresh failure each time

Two halves, one cause. The maintenance gate is coverage.indexed < coverage.indexable (maintenance.js:277), and countIndexCoverage counts every data file while buildSidecarsForTableskips quarantined ones. One file hypgrep cannot index therefore keeps coverage short forever: every tick paid a readdir plus a full listLiveDataFiles (metadata + manifest load) to rediscover there was nothing to do. And report.sidecarsFailed = built.failed + built.quarantined folded "skipped without a build" into "failed", so the partition reported a new failure on every later tick when nothing had been attempted.

Fixed: buildSidecarsForTable short-circuits on a directory read when every missing sidecar is quarantined (a file needing a build is always on disk, so this can never hide one; complete coverage still runs the full pass, because there the metadata load is what makes present a count of live files). sidecarsQuarantined is its own field on the report. Note this only became reachable in round 2: under the old compaction-only gate a failed file was never offered to a second pass, so the counter was inert.

Credit: code-review skill pass.

N4 (low, fixed). Abandoned publish scratch leaks, is billed by nothing, and nothing sweeps it

sidecar_build.js writes <sidecar>.<uuid>.tmp and renames. The failure path unlinks it, but a SIGKILL between write and rename (a shut-down daemon, an OOM kill) leaves an index-sized file behind. countDataFiles skips it for want of a .parquet suffix and round-2's measureDataDir filter skips its bytes, so hyp cache status under-reports the partition; the token is random, so each crash leaks a new file; and nothing removed them until the generation retired.

Fixed: the pass sweeps *.index.parquet.*.tmp in data/, but only past a one-hour grace window. A build takes seconds, and two writers over one cache (the daemon's tick and a hand-run hyp) is the exact case the random token exists for, so an in-flight scratch must never be pulled out from under its writer. LLP 0303 #scratch-sweep.

Credit: code-review skill pass.

Status of every round-2 residual

#	Round-2 residual	Status
S3	Unrestricted local `--regex`, the candidate blocker	Answered, accepted by design with a tripwire. Not reachable off-box at this head; see below.
S4	`exhausted` collapses two independent facts	Fixed, without changing the mirrored shape.
S5	`discoverSpoolTables` full-tree walk per query	Fixed
S6	Index worker matches leaf names, not paths	Fixed
S7	`fs.existsSync` per data file on the query path	Fixed
1	`received_at`'s justification is the server's, pasted into the client	Fixed (comment)
2	Asymmetric `scannedFiles` counter	Fixed
3	Invalid regex / over-length query exit 1	Fixed
5	A data file dereferenced without a rewrite	No longer costs what it did; carried forward, narrowed
6	The retention half of LLP 0265 T6's promised test	Fixed (both retention paths)

S3 - the one I was told to look at first, and the one I did not patch

The round-2 write-up made the severity conditional on one question: is a client host's grep_search reachable by anyone but the person at the terminal? That question is answerable from the tree, and the answer is no.

The kernel's only MCP transport is stdio. src/core/commands/mcp.js:38-41 refuses --http outright: "--http is a follow-up; only stdio is supported in V1".
stdio is local-user trust by design (mcp.js:20-22), so the caller is the same person who could type hyp query grep at the terminal, or any other command.
The daemon does not serve verbs. hyp mcp serve is its own process.

So an unbounded regex wedges a process its own author started, which is self-inflicted in exactly the way an unbounded hyp query sql is. The server's operator-only gate is the correct asymmetry, not an oversight.

I deliberately did not ship a static "catastrophic pattern" guard. The only cheap candidate is star-height detection, which has false positives on legitimate patterns ((foo\d+)+), and it would have to live in matcher.js - the module LLP 0264 #shared makes the server import too - so it would silently change what the server's operator-only regex mode accepts, to fix a client-side hazard that is not currently reachable. That trade is worse than the exposure.

What I did instead is record the determination and the tripwire, precisely, in LLP 0303 #regex-reachability, with @refs from both matcher.js (beside MAX_QUERY_LENGTH) and grep_verb.js (beside the regex property) so the next person to touch either sees it. The tripwire is one line of future work: grep_search is authClass: 'read', and createMcpServer exposes read-class verbs to a query-scoped caller on a non-stdio transport (src/core/mcp/server.js:53-60). The change that lands an HTTP transport on a client host is the change that must gate --regex, by auth class or by moving the match into the killable worker-thread seam the index build already uses.

S4 - fixed, and the shape did not have to move

GrepSearchResult's own doc comment already said exhausted means "every candidate file was walked to completion". The code said exhausted && !truncated. So this was the code disagreeing with the interface, not the interface needing to grow, and no field was added or renamed. hypaware-server mirrors the shape and computes its own value; the client's value now matches the documented meaning.

Three changes:

exhausted reports walk completion alone. truncated reports the limit alone. Both can be true.
The renderer prints each notice for its own fact, not if/else if. A search that fills its limit and aborts now says both things, which is what the shipped skill doc already teaches them to mean.
The day-descending early break no longer clears exhausted. It stops the walk only once the buffer holds hits strictly newer than every file left, which is a proof that nothing skipped could enter the answer - so reporting it as unexhausted would have fired "results may be incomplete" on every ordinary capped search, the one place that line must not appear. Two existing tests asserted the old exhausted: false there and were updated with that reasoning inline.

Recorded in LLP 0303 #completeness-signals. Server-side coordination: aligning the server's own exhausted computation with the same reading is a follow-up, not a gate - nothing renders a local and a remote answer side by side, and the field name, type and doc comment (the shared part) are unchanged. It is listed in LLP 0303 #residuals beside the server's still-pending import swap.

Residual 3 - the exit code, and why it needed a kind rather than a code

hyp query grep '(' --regex exited 1 while hyp query grep x --from 2026-8-1 exited 2, and LLP 0293 #one-contract settles that a caller's argument mistake is 2. Round 2 left it because the refusal is raised inside matcher.js, which the server imports, and a shared module must not import a CLI error class. That constraint is real; the conclusion was too strong.

The seam is a refusal kind, not a refusal code. matcher.js now raises GrepQueryError for a query the search cannot use (empty, over MAX_QUERY_LENGTH, uncompilable regex), and each surface maps that kind into its own vocabulary: the verb translates it to VerbUsageError at the boundary, an HTTP surface can map it to 400 rather than 500. The shared module still imports nothing from the CLI. Verified end to end:

$ hyp query grep '(' --regex -> exit 2 query is not a valid regular expression: ... + usage line
$ hyp query grep <1500 chars> -> exit 2 query must be at most 1024 characters + usage line
$ hyp query grep needle --from 2026-8-1 -> exit 2 (unchanged)
$ hyp query grep needle --limit 0 -> exit 2 (unchanged)
$ hyp query grep needle -> exit 0

A search that really fails is still 1, pinned by a new test.

S5, S6, S7, and the two counters

S5.discoverSpoolTables takes an optional { datasets } scope; grep passes its one dataset. The walk recurses into every generation's data/ directory, so unscoped it was a readdir of the traces, logs and metrics trees on every search to find one dataset's spool. flushAll and storage bootstrap keep the unscoped form, which is right for them: a background sweep has to reach every table anyway.
S6.element.name is a leaf's name inside its parent, not its path. Only depth-one string leaves are indexable now, which is exactly the set SCAN_COLUMNS can project. Unreachable on today's flat schema, which is why the rule is stated rather than discovered the first time a struct lands. LLP 0303 #indexable-columns.
S7. The probe is gone. A missing sidecar throws ENOENT from the open, which is the same degrade the surrounding catch already performed for every other reader failure, so the probe only added a synchronous stat per data file to an otherwise fully async walk.
scannedFiles. Moved to after the file is read whole, so the two tier counters mean the same thing (indexedFiles already counts only a file served whole; its abort path commits its buffer without counting).
received_at. The comment now says whose exclusion it is: the server's, and the client's ai_gateway_messages carries no such column, so the name is inert here. It stays in the list because the constant is shared, and dropping it would starve the server's exclusion - a wrong answer rather than a slower one.

Residual 6 - LLP 0265 T6's promised retention test

Two tests, because retention has two paths and the interesting one is not the one the LLP line suggests:

Row-level purge (the grep dataset's path, since its schema carries date): the file and its sidecar both survive, so "delete sidecars with their files" is vacuous there and the property that actually matters is the one a stale index could break. The test purges one row through the real createRetentionEnforcer, then asserts every live file is still indexed (no orphan) and that the purged row does not come back through its sidecar.
Directory reclaim (evictSourceTableByMtime, a table with no timestamp column): asserts the partition directory and every sidecar inside it are gone, which is the no-GC-code guarantee on the second path.

Residual 5 - carried forward, cheaper

A data file dereferenced without a rewrite still keeps countIndexCoverage reporting incomplete coverage until the generation retires. It now costs a directory read per tick rather than a metadata load, because N3's short-circuit runs first. Recorded in LLP 0303 #residuals.

LLP

llp/0303-grep-search-round-3-corrections.decision.md is new (0303 is the next free number across master and every remote branch; 0302 is round 2's). Extended-by: forward-refs are appended to LLP 0302's header, #build-site, #usage-exit and #residuals. Forward-refs are the mechanical edit CLAUDE.md allows on an Accepted doc; nothing 0302 settled is rewritten.

Every @ref anchor added this round resolves.

What landed

8f9b3704..9ffd056b, one commit, 16 files.

File	Change
`src/core/search/grep_service.js`	`asyncBufferFromFile` on both tiers; `exhausted` is walk completion alone; `existsSync` probe removed; `scannedFiles` counted symmetrically; spool discovery scoped
`src/core/search/grep_verb.js`	both completeness notices, not one; `GrepQueryError` mapped to exit 2
`src/core/search/matcher.js`	`GrepQueryError`; the three query refusals raise it
`src/core/search/types.d.ts`	`truncated` and `exhausted` documented as independent
`src/core/search/sidecar_build.js`	scratch sweep past a grace window; all-quarantined short-circuit
`src/core/search/index_worker_thread.js`	depth-one string leaves only
`src/core/search/searchable_columns.js`	`received_at`'s justification
`src/core/cache/maintenance.js`	`GREP_INDEX_TICK_SHARE`; `sidecarsFailed` stops folding in quarantined
`src/core/cache/spool.js`	optional `{ datasets }` scope
`src/core/cache/types.d.ts`	`sidecarsQuarantined`
`docs/CLI_REFERENCE.md`	the pattern refusals are usage errors
`llp/0303-...decision.md`	new
`llp/0302-...decision.md`	`Extended-by:` forward-refs only
`test/core/search-grep-service.test.js`	truncated-and-interrupted; two truncation assertions corrected
`test/core/query-grep-verb.test.js`	both notices render; exit 2 for an unusable pattern, 1 for a failed search
`test/core/search-sidecar-build.test.js`	scratch sweep; quarantined vs failed; both retention paths

Checked again and still sound

Re-derived at this head rather than taken from the earlier rounds: the two tiers agree on every hit; position deletes are applied by file-absolute position on both tiers and groupStart + i is the right key (we pass neither filter nor useOffsetIndex on the scan tier, so a row-group read returns exactly groupRows rows); the sub-ngramLength literal cliff is a performance property and is pinned as one; parquetFind really does forward columns into its own read; hyparquet ignores a projected name a file lacks; isAbort identity-checks signal.reason, so AbortSignal.timeout is handled; the worker's ref-while-pending, failAll, zero-length guard and pooled-Buffer copy; sidecars stay out of countDataFiles and measureDataDir, so an indexed partition does not read as growth; cwd is inside SEARCHABLE_COLUMNS, so the LLP 0105 predicate is never blinded by the projection; and no semicolons, no em dashes, JSDoc only, @import at file top with root-anchored .js specifiers.

…, and two bounds say what they hold The abandoned-publish-scratch sweep LLP 0303 added lived inside buildSidecarsForTable, which maintenance runs only when a partition's coverage is short. The crash it reclaims after leaves the sidecar unpublished, so the next tick rebuilds it and coverage goes complete again inside the sweep's own one-hour grace window: the sweep never ran again for that generation and the leak lasted its whole life. It is now sweepIndexScratch, run by the maintenance caller on every tick over a table that carries sidecars, ahead of and independent of the coverage gate. Pinned by a new test that drives the coverage-complete tick. Two docstrings claimed bounds their call paths do not hold: - The module bound "one row group plus, on the indexed tier, its index" is true on the scan tier only. parquetFind wraps whatever buffer it is handed in hyparquet's memoizing cachedAsyncBuffer, so the indexed tier's raw residency is the union of the candidate ranges it read. Nothing regressed (the previous reader held the whole file); the claim was wrong. - GREP_INDEX_TICK_SHARE reads as a per-partition allowance and is an absolute instant measured from the tick's start, so it is one window near the front of the tick. The conservative reading is kept and the docstring now says which one it is. discoverSpoolTables treated an empty datasets scope as "no dataset" while discoverCachePartitions reads the same value as "every dataset". Unreachable today; aligned with a test before a caller computes an empty list and gets a silent no-flush. LLP 0304 records both corrections with Extended-by forward-refs on LLP 0303's header, #memory-bound, #scratch-sweep and #residuals, and corrects 0303's claim that the dereferenced-data-file residual now costs a directory read: the all-quarantined short-circuit does not cover it, so it still costs a metadata load per tick. Also fixes 0303's broken link to LLP 0199 and names the --remote argument-validation exception in docs/CLI_REFERENCE.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

philcunliffe · 2026-08-24T21:01:08Z

neutral review round 4 (final) - head `9ffd056b`

Verdict: findings, four fixed and pushed. New head f3b63510, one commit, 9 files. npm test 5104 pass / 0 fail / 1 skipped and npm run typecheck are clean at the new head; query_grep_roundtrip, status_diagnostics, local_parquet_export and gateway_claude_capture are all ok.

Both passes were run and merged: a direct gh pr diff reading and the code-review skill at effort high. They overlapped on one finding and each found things the other missed. Nothing found this round makes a search return a wrong result, and no earlier round's correctness fix came apart. The two real findings are both of the shape rounds 2 and 3 kept turning up: a fix whose stated bound the call path does not hold, and this time one that also does not run in the case it was written for.

The three items I was asked to re-derive against this head are answered under Carried items, and everything still open is under Residuals for triage.

Findings

F1 (medium, fixed). The abandoned-scratch sweep never runs in the case it exists for

src/core/search/sidecar_build.js:225 (pre-fix) called sweepStaleScratch from inside buildSidecarsForTable, and src/core/cache/maintenance.js:288 runs that pass only when a partition's coverage is short. Those two cancel:

The crash LLP 0303 #scratch-sweep reclaims after is a SIGKILL between the scratch write and the rename. The rename is what publishes the sidecar, so after that crash the sidecar is missing and coverage is short.
The next tick therefore runs the pass, rebuilds that sidecar, and coverage goes complete again within seconds, which is well inside the sweep's own one-hour grace window. The sweep that did run correctly spared the scratch, because it was still young.
From then on coverage is complete, the pass never runs again for that generation, and nothing looks at the scratch again. It ages past the grace window with nothing left to reclaim it.

So the grace window, meant to delay the reclaim by an hour, suppressed it for the life of the generation: the unbounded, unbilled, once-per-crash leak #scratch-sweep set out to close. measureDataDir does not bill the bytes and countDataFiles does not count the file, so hyp cache status under-reports the partition the whole time.

Measured at 9ffd056b before the fix: index a partition to full coverage, drop a two-hour-old <sidecar>.<uuid>.tmp beside a live data file, run two maintainCache ticks, and the scratch is still there.

Fixed: the sweep is sweepIndexScratch(tableDir), exported from the same module and run by the maintenance caller on every tick over a table that carries sidecars, ahead of and independent of the coverage gate. It costs one readdir of a directory countIndexCoverage reads on the same tick anyway. The grace window is unchanged and still load-bearing (two writers over one cache is exactly what the random scratch token exists for). Pinned by a new test, a scratch left on a fully indexed partition is still reclaimed, which asserts both that no build pass ran (sidecarsBuilt === undefined) and that the scratch went anyway.

The general rule, recorded in LLP 0304: a reclaim must not be gated on the condition its own success removes.

F2 (low, fixed). The round-3 memory bound holds on the scan tier and not on the indexed one

src/core/search/grep_service.js:39 (pre-fix): "the request's memory bound really is one row group plus, on the indexed tier, its index", and LLP 0303 #memory-bound calls it "true rather than aspirational".

It is true on the scan tier. On the indexed tier parquetFind does not read through the buffer it is handed, it wraps it: file = cachedAsyncBuffer(rawFile) (hypgrep@0.5.1/src/parquetFind.js:42), a memoizing layer that holds every slice it fetches for the life of one file's search (hyparquet/src/utils.js:222). The indexed tier's raw residency is therefore the union of the candidate ranges that search read, which approaches the projected bytes of the whole file for a query the index cannot prune. A literal shorter than hypgrep's n-gram length (5) prunes to no blocks at all (queryIndex.js:82, constants.js:53), so it reads and retains every range: the case the shipped skill doc already teaches as "every candidate file is read in full".

Nothing regressed. Before the round-3 change the same wrapper sat on top of a buffer that already held the entire file, so it was strictly an improvement, and it removed a synchronous whole-file read from the loop. What did not hold is the claim.

Fixed as a statement, not as code: the module docstring and the searchIndexed comment now say which tier gets which bound, and that both are per file. Bounding the indexed tier at a row group would mean not using parquetFind's streaming entry point, or a hypgrep change to let a caller supply its own buffer wrapper; neither buys anything at the scale this cache reaches, and both cost the pruning the tier exists for. Recorded as LLP 0304 #indexed-tier-residency, with an Extended-by: forward-ref on LLP 0303 #memory-bound.

F3 (low, fixed as a docstring). `GREP_INDEX_TICK_SHARE` is one window near the front of the tick, not a per-partition allowance

src/core/cache/maintenance.js:320 passes deadlineMs: startMs + budgetMs * GREP_INDEX_TICK_SHARE, an absolute instant measured from the tick's start, while the constant's docstring reads as an allowance handed out per partition. Found by the code-review pass, which added the fact that makes it bite: the neediest-first walk at maintenance.js:170 ranks every dataset together, so a busy logs or traces partition compacting first can spend the whole 25% window before the first grep partition's build pass is reached. Every grep partition after that indexes exactly one file per tick (the mandatory first attempt) and defers the rest.

I did not change the behavior, and the reasoning is under Residual 3. The docstring now describes the mechanism it actually implements, so nobody reads a larger bound out of it than the code holds.

F4 (low, fixed). `discoverSpoolTables` read an empty scope as "no dataset" where its neighbour reads "every dataset"

src/core/cache/spool.js:522 (pre-fix): opts.datasets ? opts.datasets.map(...) : [root], so { datasets: [] } walked nothing and returned []. The adjacent scoping API discoverCachePartitions (partition.js:187) uses scope.datasets && scope.datasets.length > 0, i.e. [] means all. Verified at 9ffd056b: over a two-dataset tree, unscoped returns 2, ['ds_a'] returns 1, and [] returns 0.

Unreachable today (grep always passes exactly one dataset), and both APIs are routinely handed the same computed list. One reading [] as "all" while the other reads it as "nothing" would make a caller whose list came out empty walk every partition and flush no spool, and that surfaces as a query answering from a stale cache rather than as an error. Aligned, with a test.

Credit: code-review pass.

Carried items I was asked to re-derive

LLP 0303 #regex-reachability still holds at this head. Re-derived, not taken from round 3:

src/core/commands/mcp.js:38-41 still refuses --http outright.
mcp.js:20-22 still states stdio as local-user trust, and buildOperationContext still derives callerCwd from the process cwd.
createMcpServer has exactly one caller in the tree (src/core/commands/mcp.js:51).
The daemon still serves no verbs. The only two http.createServer call sites in src/ are the OTLP ingest listener (src/core/otlp/server.js:53) and the hyp remote login OIDC loopback receiver (src/core/remote/loopback.js:192); neither references ctx.verbs, runTool or verbs.list().
Both tripwire @refs (matcher.js beside MAX_QUERY_LENGTH, grep_verb.js beside the regex property) still resolve.

Nothing changed, so this is not a blocker-class finding.

Round-2/3 "residual 5" (a data file dereferenced without a rewrite): the round-3 claim that it is "now cheaper" does not hold. See Residual 4. Not closable; corrected in the record instead.

S5, the per-query discoverSpoolTables full-tree walk: closed. The round-3 fix is correct. grep_service.js:193 passes { datasets: [GREP_DATASET] }, the dataset name is the directory name under datasets/ (cacheTablePath), and the background callers (flushAll, storage bootstrap) keep the unscoped form, which is right for them. The empty-scope hole in the same function is F4 above and is fixed.

Server exhausted alignment: not touched, as instructed. Stated precisely under Residual 1.

Residuals for triage

Residual 1: the server's own `exhausted` computation

What diverges. Round 3 made this repo's exhausted mean walk completion alone, unfolded from truncated (grep_service.js:585). hypaware-server mirrors the GrepSearchResult shape and computes its own value; whether it agrees with the field's documented meaning is out of tree and unverified from here.
Conditions and who notices. Only on --remote, and only through the two stderr notices. A remote search that both filled its limit and was cut short by the server's deadline could print one notice where the local path prints two. The field name, type and doc comment (the shared part) are unchanged, so nothing structural breaks and no hit is wrong.
Does anything render local and remote side by side? No. runVerbCommand:100-128 takes exactly one of the two branches and render runs once, on whichever result came back. There is no surface that unions or diffs the two.
Fix and blast radius. A change in hypaware-server's own grep_search, plus a note on server LLP 0178's lineage. Zero code change in this repo.
My read: preference. A follow-up issue on hypaware-server. It cannot produce a wrong answer here, only a possibly-less-precise notice on the remote path, and the divergence is invisible to any single invocation.

Residual 2: the server's import swap onto this repo's shared modules

What breaks. Nothing today. Until the swap lands, two repositories hold two copies of SEARCHABLE_COLUMNS / SCAN_COLUMNS / the matcher, and LLP 0264 #shared's "no tier can surface a match another tier cannot" rests on them being kept equal by hand. GrepQueryError (round 3) joins the set of names the swap has to pick up.
Who notices. Nobody, until someone edits one copy. Then a query answers differently local versus remote, silently.
Fix and blast radius. A hypaware-server change; nothing here.
My read: preference. The actual release gate (server T7: Wizard fork phase + returning-gate amendment #364, the grep_search displacement) is merged, which is what LLP 0264 #open was about.

Residual 3: the build pass's tick share is a front-of-tick window, not a per-partition allowance

What happens.maintenance.js:320 uses startMs + budgetMs * 0.25. The neediest-first walk (maintenance.js:170) ranks all datasets together, so a busy logs or traces partition compacting first can spend that window before any grep partition's build is reached. Each grep partition after that indexes exactly one file per tick and defers the rest.
What does NOT break. Coverage still advances monotonically: the mandatory-first-attempt guarantee means at least one file per grep partition per tick, so the hyp cache status line "maintenance indexes them" stays actionable. This is not round 2's finding 7 again (advice naming work that will never run); the work does run, just slowly.
Who notices and when. An operator on a large, busy cache watching grep index: N of M crawl, with greps over deep history staying on the scan tier longer than expected. Needs both many unindexed grep files and enough non-grep compaction ahead of them to eat the window.
Fix and blast radius. Literally one token: Date.now() + budgetMs * GREP_INDEX_TICK_SHARE. But that recreates round 3's N1 starvation one partition wider: K grep partitions take K shares, and the loop's own budget break then cuts the non-grep partitions behind them. So the two readings trade grep coverage speed against the rest of the maintenance loop, and neither is obviously right without a measurement on a real cache.
My read: preference. The code holds the conservative side of the trade and the docstring now says so. If triage wants it flipped, the change is one token plus a test, but it is a scheduling decision with an LLP attached, which is why I did not flip it in the last round.

Residual 4: a data file dereferenced without a rewrite, and round 3's cost claim for it

What breaks. An unreferenced .parquet in a live generation's data/ (a killed append, a conflict-aborted commit) is counted by countIndexCoverage (a raw readdir, maintenance.js:1609) and never by buildSidecarsForTable, which walks listLiveDataFiles. Coverage is therefore permanently short and the build gate fires on every tick.
Why the round-3 mitigation does not cover it. LLP 0303 #residuals says it "now costs a directory read per tick rather than a metadata load, because #build-share's short-circuit runs first". That short-circuit fires only when every missing sidecar is quarantined, and a dereferenced file has never been offered to a build, so it is not in the quarantine ledger: scanForBuildable reads it as buildable, the short-circuit is skipped, and the pass pays a full listLiveDataFiles (metadata plus manifest-list plus manifest) every tick to build nothing.
Measured at this head. With one orphan copied into a live generation's data/ and compaction held off, three consecutive maintainCache ticks each report sidecarsBuilt: 0 (the pass ran), where the same partition without the orphan reports the field absent (the gate skipped it).
Who notices. An operator: hyp cache status shows that partition permanently under-indexed with advice it can never satisfy. Ends when the generation retires. Never a wrong search result: the file is not in the table, so nothing greps it either way.
Fix and blast radius. Either give the gate a live-file denominator (a metadata load per grep partition per tick, the exact cost the readdir gate exists to avoid, and it touches every grep partition on every tick), or add a separate orphan reaper for data/ (its own grace window, and an argument about racing a writer). Both are larger than the leak.
My read: preference, and the one I would rank first. It is the only residual with a permanently-wrong operator-facing line attached. Not a blocker: bounded, self-clearing at generation retirement, no effect on any answer. The false cost claim is corrected in LLP 0304 #residuals with an Extended-by: forward-ref on LLP 0303 #residuals, so the record no longer overstates the mitigation.

Residual 5: `--remote` skips the verb's operation-level argument rules

What breaks.runVerbCommand:100-128 takes the remote branch before verb.operation, so dayBound, the inverted-window check and the GrepQueryError to VerbUsageError mapping never run on --remote. hyp query grep x --remote T --from 2026-8-1 gets whatever the server says instead of the local exit 2 plus usage line. Schema-level rules still run locally, because the codec runs first: unknown flags and --limit 0 are refused on both paths.
Who notices. A script that treats exit 2 as "you typed it wrong" and gets whatever the server maps a bad day to.
Fix and blast radius. A verb-level pre-remote validation hook in the codec (verb_command.js plus hypaware-plugin-kernel-types.d.ts), which touches every verb, not just grep. query sql has the same shape today (its read-only check is also operation-level).
My read: preference. The verb framework's shape rather than grep's, and the server is the right authority for a remote call. docs/CLI_REFERENCE.md now names the exception instead of stating the local refusals unconditionally.

Residual 6: the sidecar is read synchronously, whole, once per data file

What happens.grep_service.js:449 opens the sidecar through io.reader, which is the cache resolver's fs.readFileSync (resolver.js:26-29). Round 3 removed the existsSync probe on the grounds that it "added a synchronous stat per data file to an otherwise fully async walk"; the walk is not fully async on that line, and the read that replaced the probe is much larger than the stat.
What does NOT break. Removing the probe strictly removed work, so this is not a regression, and both LLP 0303 #memory-bound and the code deliberately keep the sidecar on the resident reader (it is the pruning structure, read in many small random ranges, and a fraction of the file it indexes). The loop it blocks belongs to the process running the search: hyp query grep and hyp mcp serve are their own processes and the daemon never serves grep.
Fix and blast radius. Handle-backing the sidecar too, which would trade one sequential read for many small random ones through hypgrep's queryIndex, probably for the worse. Or nothing, and a corrected rationale line.
My read: preference, lowest of the six. Listed only so the rationale is not re-litigated as a fact.

What landed

9ffd056b..f3b63510, one commit.

File	Change
`src/core/search/sidecar_build.js`	`sweepStaleScratch` becomes the exported `sweepIndexScratch(tableDir, log)`; the build pass no longer owns it
`src/core/cache/maintenance.js`	the sweep runs before and outside the coverage gate; `GREP_INDEX_TICK_SHARE` docstring says which deadline it makes
`src/core/search/grep_service.js`	the memory bound is stated per tier: one row group on the scan tier, the candidate ranges on the indexed one
`src/core/cache/spool.js`	an empty `datasets` scope means every dataset, matching `discoverCachePartitions`
`docs/CLI_REFERENCE.md`	the day and pattern refusals are local; `--remote` applies the server's rules
`llp/0304-...decision.md`	new: `#scratch-sweep-site`, `#indexed-tier-residency`, `#residuals`
`llp/0303-...decision.md`	`Extended-by:` forward-refs on the header, `#memory-bound`, `#scratch-sweep`, `#residuals`; broken LLP 0199 link fixed
`test/core/search-sidecar-build.test.js`	the sweep is driven through its new entry point; a scratch on a fully indexed partition is reclaimed
`test/core/cache-spool-append-durability.test.js`	an empty scope is not an empty answer

Numbering: 0304 is the next free number across master and every remote branch (0301 bounded-compaction-resettle, 0302 round 2, 0303 round 3). Author neutral, matching 0302 and 0303. Nothing 0303 settled is rewritten; forward-refs are the mechanical edit CLAUDE.md allows, and the LLP 0199 link fix is a broken link.

Checked again and still sound

Re-derived at this head rather than carried: the day-descending early break against hits[hits.length - 1].date (safe because the buffer is trimmed to budget first, so the comparison is against a hit one rank below the answer, and a ''-dated hit never triggers it); truncation in sort order on both the shared buffer and the indexed tier's per-file buffer; position deletes applied by file-absolute position on both tiers, with groupStart + i on the scan tier and range.rowStart + i from parquetFind on the indexed one; LocalOnlyListUnreadableError propagating out of both tiers rather than degrading a sidecar that is fine; isAbort identity-checking signal.reason, so AbortSignal.timeout is handled; the indexed tier committing its buffer on abort and not counting the file; the limit schema (integer, minimum: 1) surviving toJsonSchema and being enforced for an MCP caller by validateToolArguments to coerceValue, so limit: 0 over the wire is refused and not answered with 50; hyparquet ignoring a projected column a file lacks, which is what makes received_at inert here; the sub-ngramLength literal falling back to every block (a full scan, never a missed hit); asyncBufferFromFile opening a fresh createReadStream per slice, so the walk leaks no file descriptor across thousands of data files; the worker's ref-while-pending, failAll on both error and exit, the zero-length-index guard and the pooled-Buffer copy; sidecars and the publish scratch staying out of countDataFiles and measureDataDir, so an indexed partition does not read as growth; and no semicolons, no em dashes anywhere in the diff, JSDoc only, @import at file top with root-anchored .js specifiers. Every @ref anchor added across all four rounds resolves, and the only broken markdown link in llp/ (LLP 0303 to LLP 0199) is fixed.

philcunliffe · 2026-08-24T21:28:19Z

Triage rung: every residual from review round 4 re-derived at head f3b6351 and classified non-blocking, so this PR can merge safely.

The one candidate blocker (unrestricted local --regex, round 2's S3) was re-derived independently against the tree: grep_search is not reachable off-box (hyp mcp serve refuses --http, stdio is local-user trust, the daemon serves no verbs, and the only HTTP listeners in src/ are the OTLP ingest and the OIDC loopback), so an unbounded regex is self-inflicted, the same class as an unbounded hyp query sql. The tripwire for a future HTTP transport is recorded in LLP 0303 #regex-reachability. The remaining rounds' pattern of docstrings claiming bounds the code did not hold was also re-checked: no such claim survives at this head, and the 59 search-stack tests pass.

The six deferred findings, each with conditions, fix and blast radius, are enumerated in #995.

bgmcmullenand others added 18 commits August 19, 2026 10:38

Merge T6 sidecar build into the T7 base
c06017a

Integrate the grep search stack (LLP 0265 T4-T7)
669a633

bgmcmullen added the neutral:adopt Foreign PR adopted into neutral's reconcile scope label Aug 21, 2026

philcunliffe added the neutral:adopted Adoption completion record: merged while carrying neutral:adopt (LLP 0031) label Aug 21, 2026

philcunliffeand others added 6 commits August 21, 2026 20:01

philcunliffeand others added 2 commits August 24, 2026 19:06

philcunliffe mentioned this pull request Aug 24, 2026
Follow-up: deferred review findings from PR #984 #995
Open

philcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Aug 24, 2026

bgmcmullen added this pull request to the merge queue Aug 24, 2026

Merged via the queue into master with commit 4452bf6Aug 24, 2026
8 checks passed

bgmcmullen deleted the grep/integration branch August 24, 2026 21:53

This was referenced Aug 25, 2026
Follow-up: deferred review findings from PR #951 #971
Open
Follow-up: PR #954 takes #953 and moves the sidecarPathFor import (decided on-thread) #979
Open

Navigation Menu

Uh oh!

hyp query grep: the full search stack, integrated on master (LLP 0265 T4-T7) - #984

hyp query grep: the full search stack, integrated on master (LLP 0265 T4-T7)#984
bgmcmullen merged 28 commits into
masterfrom
grep/integration

bgmcmullen commented Aug 21, 2026•
edited by philcunliffe
Loading

Uh oh!

philcunliffe commented Aug 24, 2026

Uh oh!

philcunliffe commented Aug 24, 2026

Uh oh!

philcunliffe commented Aug 24, 2026•
edited
Loading

Uh oh!

philcunliffe commented Aug 24, 2026

Uh oh!

philcunliffe commented Aug 24, 2026

Uh oh!

philcunliffe commented Aug 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

bgmcmullen commented Aug 21, 2026• edited by philcunliffe Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this is

Trying it locally

What it adds

Integration fixes this branch carries

Verification

Note on coverage

Uh oh!

philcunliffe commented Aug 24, 2026

Review round 1 at 4e6299b

Fixed in this round

Open, needs a human

What I checked and found sound

Uh oh!

philcunliffe commented Aug 24, 2026

Uh oh!

philcunliffe commented Aug 24, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

neutral review round 2 (final) - head 8613a637

The six round-1 open findings

6. LLP hygiene - fixed

7. A partition at the compaction floor never indexed - fixed (was the real bug of the six)

8. Unbudgeted build - fixed for the budget; the whole-file read is unchanged and defensible

9 and 10. --from/--to - both fixed

11. Server coordination - narrowed, and no longer a gate

New findings this round

Residuals for the triage rung

What landed

Second pass: the code-review skill (run in an isolated worktree)

S1 (major, fixed) - the brute scan materialized a whole data file

S2 (minor by the reviewer's grading, fixed - I read it as the same class as findings 9/10) - an unusable --limit was answered, not refused

S3 (major, left for triage - the one I would look at first) - unrestricted --regex on the local path

S4 (minor, left for triage) - exhausted collapses two independent facts

S5 (minor, left for triage) - a full-tree walk prepended to every grep

S6 (nit, left for triage) - the index worker matches leaf names, not paths

S7 (nit, left for triage) - fs.existsSync per data file on the query path

Checked and found sound (recording so it is not re-litigated)

Final state

Uh oh!

philcunliffe commented Aug 24, 2026

neutral review round 3 - head 8f9b3704

New findings this round

N1 (medium, fixed). The build pass inherits the tick's tail and can starve every partition behind it

N2 (medium, fixed). The round-2 memory-bound fix bounded the decoded rows and not the bytes

N3 (low, fixed). A quarantined file bought a metadata load every tick, forever, and reported a fresh failure each time

N4 (low, fixed). Abandoned publish scratch leaks, is billed by nothing, and nothing sweeps it

Status of every round-2 residual

S3 - the one I was told to look at first, and the one I did not patch

S4 - fixed, and the shape did not have to move

Residual 3 - the exit code, and why it needed a kind rather than a code

S5, S6, S7, and the two counters

Residual 6 - LLP 0265 T6's promised retention test

Residual 5 - carried forward, cheaper

LLP

What landed

Checked again and still sound

Uh oh!

philcunliffe commented Aug 24, 2026

neutral review round 4 (final) - head 9ffd056b

Findings

F1 (medium, fixed). The abandoned-scratch sweep never runs in the case it exists for

F2 (low, fixed). The round-3 memory bound holds on the scan tier and not on the indexed one

F3 (low, fixed as a docstring). GREP_INDEX_TICK_SHARE is one window near the front of the tick, not a per-partition allowance

F4 (low, fixed). discoverSpoolTables read an empty scope as "no dataset" where its neighbour reads "every dataset"

Carried items I was asked to re-derive

Residuals for triage

Residual 1: the server's own exhausted computation

Residual 2: the server's import swap onto this repo's shared modules

Residual 3: the build pass's tick share is a front-of-tick window, not a per-partition allowance

Residual 4: a data file dereferenced without a rewrite, and round 3's cost claim for it

Residual 5: --remote skips the verb's operation-level argument rules

Residual 6: the sidecar is read synchronously, whole, once per data file

What landed

Checked again and still sound

Uh oh!

philcunliffe commented Aug 24, 2026

Uh oh!

bgmcmullen commented Aug 21, 2026•
edited by philcunliffe
Loading

Review round 1 at `4e6299b`

philcunliffe commented Aug 24, 2026•
edited
Loading

neutral review round 2 (final) - head `8613a637`

9 and 10. `--from`/`--to` - both fixed

Second pass: the `code-review` skill (run in an isolated worktree)

S2 (minor by the reviewer's grading, fixed - I read it as the same class as findings 9/10) - an unusable `--limit` was answered, not refused

S3 (major, left for triage - the one I would look at first) - unrestricted `--regex` on the local path

S4 (minor, left for triage) - `exhausted` collapses two independent facts

S7 (nit, left for triage) - `fs.existsSync` per data file on the query path

neutral review round 3 - head `8f9b3704`

neutral review round 4 (final) - head `9ffd056b`

F3 (low, fixed as a docstring). `GREP_INDEX_TICK_SHARE` is one window near the front of the tick, not a per-partition allowance

F4 (low, fixed). `discoverSpoolTables` read an empty scope as "no dataset" where its neighbour reads "every dataset"

Residual 1: the server's own `exhausted` computation

Residual 5: `--remote` skips the verb's operation-level argument rules