Hoist the shared grep-search modules into src/core/search/ and publish them as ./core/search

[philcunliffe]

Root cause

LLP 0264 #shared requires the client and the server to agree byte for byte on what grep search looks at and what a hit is: "zero hits" only means something if both sides searched the same columns. The server already depends on this package ("hypaware": "file:../hypaware"), but the exports map had ./core/query and nothing search-shaped, so there was no entry for the server to import through and two drifting copies were the only available option.

The fix

New src/core/search/, carrying the pieces both repos must share and nothing else:

• searchable_columns.js: SEARCHABLE_COLUMNS (the allowlist, insertion order meaningful so the content column leads a hit's snippets) and SCAN_COLUMNS (the wider brute-scan projection, its extras derived from the readers). Verbatim semantics from the server's src/search/searchable-columns.js.
• matcher.js: compileMatcher(query, regex) returning hypQuery / test / locate / rowTest, the snippet-window constants (SNIPPET_BEFORE, SNIPPET_AFTER, MAX_MATCH_COLUMNS, MAX_QUERY_LENGTH), and makeSnippet. Hoisting the window cut alongside its constants is deliberate: leaving the arithmetic behind would duplicate exactly what the constants are shared to prevent, and T4/T5 need it locally.
• types.d.ts: GrepSearchHit, GrepSearchResult, GrepSearchMatcher.
• index.js: the hypaware/core/search surface.

package.json gains "./core/search" shaped exactly like ./core/query. Semantics are the server's originals unchanged, so its follow-up import swap is a no-op diff rather than a port; the @refs point at LLP 0264 #shared and LLP 0265 #out-of-scope (no config knob for indexed columns).

received_at stays in SCAN_COLUMNS even though the client's ai_gateway_messages has no such column: hyparquet's object mode skips a name absent from the file, and the whole point is that the two projections do not drift.

Nothing imports these modules yet, so there is no runtime behavior change anywhere. T4 (the local grep service) and T5 (the verb) are what use them.

The test that proves it

Three suites under test/core/, all of which fail on master (the modules and the export entry do not exist) and pass here:

• search-searchable-columns.test.js: the allowlist is exactly the shared ten in order, the bulk machinery columns (system_text, tools, attributes, raw_frame, status) stay out, every searchable column really exists on AI_GATEWAY_MESSAGE_COLUMNS, and the scan projection is the allowlist plus the five reader columns with no duplicates.
• search-matcher.test.js: literal vs regex compile (both case-insensitive), locate offsets including the no-match degradation and the zero-width regex match, empty/oversized query refusal, rowTest matching only through allowlisted columns (system_text cannot produce a hit), and snippet windows at the head, the tail, mid-buffer, and shorter-than-the-window.
• search-exports.test.js: the ./core/search entry matches the ./core/query shape, src/ and types/ are both in the published file set, and import('hypaware/core/search') resolves and carries the eight shared names.

Verification

• npm test: green, 4501 pass / 0 fail / 1 skipped.
• npm run typecheck: clean.
• npm run build:types: emits types/core/search/{index,matcher,searchable_columns}.d.ts; the root-anchored ../../../src/core/search/types.js specifier resolves identically from src/ and from the generated types/ tree.
• npm pack --dry-run: includes src/core/search/ (4 files, types.d.ts among them) and types/core/search/.

Fixes#872

[philcunliffe]

Neutral review round: PR #876 @ 5d40f422

Verdict: approve after fixes. The hoist itself is right, the modules are the ones LLP 0264 #shared names, and the three test suites are honest. But two of the three "pieces both sides must agree on" did not actually work as advertised at 5d40f422: one allowlisted column could never produce a hit, and the hit shapes were not importable by anyone. Both are fixed on the branch; the head is now f74de037.

7 findings: 1 high, 2 medium, 4 low. 5 fixed, 1 declined, 1 folded into another fix.

---

1. high - tool_args is in the allowlist but can never match (FIXED)

src/core/search/searchable_columns.js:26 lists tool_args, but tool_args is a JSON column: hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js:84 declares { name: 'tool_args', type: 'JSON' }, src/core/cache/iceberg/schema.js:194 maps JSON to iceberg variant, and coerceForIceberg (src/core/cache/iceberg/schema.js:226) hands the raw JS value straight through. So it reads back from parquet as an object, not a string.

anySearchableCell (src/core/search/matcher.js:110 at the reviewed SHA) gated on typeof value === 'string', so the object form was dropped silently. Net effect: searching for a file path or a Bash command inside a tool call answers zero hits on every scan path, while the column is still decoded on every brute scan (it is in SCAN_COLUMNS) purely to be ignored. The indexed tier reads the column's own text and would answer otherwise, which is exactly the tier drift this module exists to prevent.

The repo already has the precedent: parseMaybeJson in hypaware-core/plugins-workspace/ai-gateway-graph/src/tool_facets.js:530, documented as "tool_args may arrive parsed or as a JSON string, like everywhere else in the contract".

Fix: new exported cellText(value) in matcher.js renders a cell (string as-is, object via JSON.stringify, everything else as '') before testing; anySearchableCell runs through it. cellText is exported from index.js so T4's per-column snippet path uses the same rendering the row predicate does. New test asserts both the object and the string form of tool_args hit, that a non-matching one does not, and that attributes stays excluded whatever shape it holds.

2. medium - the hit shapes are not reachable by any consumer (FIXED)

LLP 0264 #shared hoists three things, and the third is "the GrepSearchHit / GrepSearchResult shapes". They were not reachable. src/core/search/index.js re-exported only runtime values, and an exports map blocks every subpath it does not name, so there was no specifier for the server to import through. Verified from a scratch consumer package (symlinked, moduleResolution: nodenext): import type { GrepSearchHit } from 'hypaware/core/search/matcher.js' gives TS2307 Cannot find module. test/core/search-exports.test.js:44 pinned the exact 8-name export list, locking the omission in.

Fix: new "./core/search/types.js" exports entry with a types condition only, pointing at ./src/core/search/types.d.ts - the hand-written declaration in src/, not a build output, because tsc does not copy a .d.ts input into the generated types/ tree (npm run build:types emits index/matcher/searchable_columns.d.ts and nothing else). Verified positively: the same scratch consumer now compiles import type { GrepSearchHit, GrepSearchResult, GrepSearchMatcher } from 'hypaware/core/search/types.js' clean. New test pins the entry and asserts its target exists on disk.

3. medium - a malformed regex escapes as a raw SyntaxError (FIXED)

src/core/search/matcher.js:54 (reviewed SHA) called new RegExp(query, 'i') unguarded, in a function whose own docblock says "Validation lives here rather than in each caller so every serving surface enforces the identical rule". hyp query grep --regex '(' throws Invalid regular expression: /(/i: Unterminated group, which the serving surface cannot distinguish from an internal fault: a 500 for what is plainly a bad request.

Fix:compileRegex wraps the construction and rethrows as query is not a valid regular expression: <detail>, the same Error shape as the empty and oversized refusals. Test covers ( and a{2,1}, and asserts the same ( is fine as a literal.

4. low - literal offsets index a lowercased copy, not the value (FIXED)

Literal locate computed value.toLowerCase().indexOf(...) and makeSnippet sliced the originalvalue at that index. Any character whose lowercase form is longer (İ U+0130 lowercases to two code units) shifts every later offset by the accumulated delta, so the snippet window opens mid-word. The reported length was the query's length rather than the matched region's, and literal mode accepted values regex mode would not, so the two modes disagreed on the same cell.

5. low - every cell lowercased up to three times per row (FIXED, same change)

rowTest, test, and locate each allocated a full lowercase copy of the cell, on the multi-megabyte bodies the makeSnippet docblock explicitly anticipates.

Fix for 4 and 5: literal mode now compiles new RegExp(escapeLiteral(query), 'i') and shares the one compiled regex across test / locate / rowTest. Offsets index the original value, the matched length is the real one, and no per-cell copy is allocated. hypQuery still hands hypgrep the raw literal string for index pruning, and the literal miss still degrades to { index: 0, length: query.length } (regex to length: 1), so the existing pinned behavior is unchanged. Test asserts offsets under İİİ needle and that a.c does not match abc.

6. low - snippet edges can cut a surrogate pair (FIXED)

value.slice(start, end) cut at arbitrary code-unit offsets, so a match with an emoji at the 80-before / 160-after boundary yielded a lone surrogate in a snippet that gets JSON-serialized and rendered in a terminal.

Fix:makeSnippet nudges start forward and end back off a low surrogate. Test asserts snippet.isWellFormed() across an all-emoji buffer.

7. low - SEARCHABLE_COLUMNS / SCAN_COLUMNS exported mutable (DECLINED)

Raised as: any importer could SEARCHABLE_COLUMNS.add('system_text') and widen what every tier searches, defeating the byte-for-byte agreement, where the neighbouring AI_GATEWAY_MESSAGE_COLUMNS uses Object.freeze.

Declined, deliberately. Object.freeze on a Set does not prevent .add/.delete (they go through internal slots), so the fix would not actually protect the member it is aimed at without a throwing wrapper. And freezing SCAN_COLUMNS narrows its inferred type to readonly string[], which will fight the columns: string[] parameter T4 passes it to. The real guard here is the test suite pinning the exact list, which is already in place (test/core/search-searchable-columns.test.js). Worth revisiting only if T4 finds a shape where it is free.

---

Checks

• npm test: green, 4508 pass / 0 fail / 1 skipped (up from 4501 pass; 7 new assertions' worth of tests).
• npm run typecheck: clean.
• npm run build:types: clean, emits types/core/search/.
• Consumer resolution probe: hypaware/core/search and hypaware/core/search/types.js both resolve for an external package under moduleResolution: nodenext.

Head after fixes: f74de037.

[philcunliffe]

Review round: f74de037 (neutral, code-review)

Verdict: findings (7 total: 3 medium, 4 low). 4 fixed and pushed as 4b2e916a; 3 left open with reasons below.

The exports/packaging half of the PR is sound: ./core/search matches the ./core/query shape, npm run build:types emits types/core/search/, src/ and types/ are both in the published file set, and the root-anchored @import specifier resolves identically from src/ and from the generated types/ tree. Every finding is in matcher.js / searchable_columns.js, and each was reproduced by running the shipped code.

Fixed in 4b2e916a

1. medium: makeSnippet was unbounded when the match itself is long (src/core/search/matcher.js:143 at the reviewed head)
The window was SNIPPET_BEFORE/SNIPPET_AFTER around found.index, but found.length came straight from m[0].length and was added to end unclamped, against a doc directly above it that promises "a bounded window around the first match, never the full column value: a matched message body can be megabytes." Reproduced: with a 4,000,006-char content_text, makeSnippet(value, compileMatcher('.*needle.*', true)) returned 4,000,006 chars (and 2,000,089 for needle[\s\S]*). .*needle.* is exactly what an rg-trained user types, and that snippet is JSON-serialized into the hit and printed to a terminal, up to MAX_MATCH_COLUMNS per hit times the limit. Literal mode was already safe (capped by MAX_QUERY_LENGTH); only regex mode was unbounded.
Fix: the matched run is clamped to SNIPPET_AFTER before the window opens. The same 4MB cell now snippets to 323 chars (greedy) / 406 chars (open-ended). Test: a snippet stays bounded when the match itself is unbounded.

2. medium: test/locate/makeSnippet did not route through cellText, so they disagreed with rowTest (src/core/search/matcher.js:94)
rowTest called cellText per cell, but test: (value) => re.test(value) and locate took the raw value. Reproduced: for row = { tool_args: { file_path: 'a.js' } } and query file_path, matcher.rowTest(row) === true while matcher.test(row.tool_args) === false, and makeSnippet(row.tool_args, matcher) threw TypeError: value.slice is not a function. The natural consumer loop (if (!rowTest(row)) continue; for (col of SEARCHABLE_COLUMNS) if (test(row[col])) ...) therefore either emits a hit with an empty matches array or crashes, on exactly the column the row matched through, and this module exists so the two repos cannot write that loop differently.
Fix: all four entry points render the cell through cellText; GrepSearchMatcher.test/locate widen to unknown in types.d.ts. Test: test, locate and makeSnippet agree with rowTest on a JSON cell.

3. medium: cellText's JSON.stringify rendered escapes, so ordinary literal queries could never match inside tool_args (src/core/search/matcher.js:123)
The serialized text carries the escapes, not the characters. Reproduced on { command: 'cd /repo\nnpm test', path: 'C:\Users\me' }: rowTest returned false for the literal query C:\Users\me and for the literal multi-line command, even though grepping for a Windows path or a shell command inside a tool call is the motivating use case the comment names.
Fix: a decoded JSON cell is walked to its keys and primitive leaves, joined one per line, instead of serialized. Windows paths, embedded newlines, and quoted strings now match, and key names stay searchable. Cycles now cost the revisiting branch rather than the whole cell. Test: a JSON cell is searched as its decoded text, escapes and all.
Known residue, deliberately not fixed here: a cell that arrives already serialized (the paths that carry tool_args verbatim as a JSON string) is still matched as the text it is, so a query containing a JSON escape still misses that form. Nothing in cellText knows the column name, and parsing every JSON-looking string would change what a content_text holding a JSON document matches. Documented in the function comment; T4/T5 hold the column name and are where it should be settled with the server.

4. low: the exported Set was mutable while SCAN_COLUMNS is a load-time snapshot of it (src/core/search/searchable_columns.js:28)
The doc says "The set is a constant, not configuration," but Object.freeze does not reach Set.prototype.add/delete and nothing froze it anyway. Reproduced: after SEARCHABLE_COLUMNS.add('system_text'), rowTest({system_text: 'a needle'}) returned true process-wide while SCAN_COLUMNS.includes('system_text') stayed false -> searchable on the row predicate, silently zero on the scan that never decodes the column.
Fix:constantSet() replaces add/delete/clear with a throwing implementation and freezes the instance; still a real Set, so the shared surface is unchanged. Test: the allowlist cannot be mutated out from under the scan projection.

Open (not fixed, with reasons)

5. low: cellText re-renders every JSON cell on every scanned row (src/core/search/matcher.js:99)
anySearchableCell runs cellText over all 10 allowlisted columns per row and short-circuits only on a match, so every non-matching row (the overwhelming majority of a brute scan) pays a full render of tool_args, which for a Write/Edit call carries the file body. This is the same whole-buffer copy the comment at line 20 cites as the reason literal mode avoids value.toLowerCase(). Not fixed because the obvious fix (test each leaf as the walk reaches it, short-circuiting) reintroduces finding 2 in miniature: a query spanning a leaf boundary would then match rowTest but not test/makeSnippet. The projection and the scan loop live in T4; that is where the cost is measurable and where a cheaper form can be chosen without splitting the semantics.

6. low: regex mode compiles without the u flag (src/core/search/matcher.js:52)
Reproduced: compileMatcher('\p{L}+', true).test('abc') === false while .test('xx p{L} yy') === true; \u{1F600} compiles without complaint and matches nothing sensible. compileRegex exists so a bad pattern is a clean 400 rather than a 500, but these patterns take the silently-wrong path instead of the refusal path. Not fixed because this module's whole purpose is byte-identical semantics with the server's matcher, adding u also turns some currently-compiling patterns into refusals, and the server's flags are not visible from this repo. Action for a human: check the server's compileMatcher flags and either add u on both sides in one change, or record the omission as intentional in the comment.

7. low (flag for T6): cellText's rendering of tool_args is unlikely to equal what a hypgrep sidecar indexes (src/core/search/matcher.js:105)
tool_args is an Iceberg variant (schema.js maps JSON -> variant), i.e. binary in parquet, whereas the scan matches a rendering this module chooses. The comment asserts "the indexed tier, which reads the column's own text, answers otherwise", but if hypgrep's index over a variant column renders differently (or not at all), then once T6 builds sidecars, parquetFind prunes away a file whose only match is in tool_args and the same query that works today returns nothing after compaction: the LLP 0264 #shared drift in the opposite direction. hypgrep is not yet a dependency, so nothing here can catch it. Worth verifying before T6 lands. Note the leaf rendering in fix 3 changes what this has to agree with, in the direction of decoded text.

Checked and cleared

Surrogate-pair trimming in makeSnippet (fuzzed 40 offsets, zero malformed snippets); escapeLiteral covers the standard metacharacter set; the shared RegExp carries no g/y flag so lastIndex is not a hazard; prepare builds types/ before pack; src/core/search/types.d.ts is correctly pointed at in src/ rather than a never-generated types/ copy; the @refs to LLP 0264 #shared and LLP 0265 #out-of-scope still describe the code they sit on.

Verification of the fix commit

npm test green (4512 pass, 0 fail, 1 skipped, up from 4501 with the 4 added/updated tests), npm run typecheck clean, npm run build:types still emits types/core/search/{index,matcher,searchable_columns}.d.ts. All four fixes re-reproduced against the committed tree.

[philcunliffe]

Triage: residual findings at 4b2e916a are all non-blocking

The review-round cap is exhausted with 3 findings open from the last round (plus 1 documented residue). Each was judged against the code at the head, not the review's severity label. None can cause a production defect at this head: nothing outside test/ imports hypaware/core/search yet, so no runtime path executes the matcher, and each open item is either a deferred optimization whose cost lives in T4, a coordinated cross-repo flag decision (u flag, needs the server's flags in hand), or a prospective T6 risk (hypgrep is not yet a dependency).

Deferred to follow-up issue #909, with per-finding file:line, the reason each is non-blocking, and the concrete condition under which each becomes actionable.

The PR is safe to merge; a human holds the merge decision as always.