Surfaces and proof: status coverage, the skill learns grep, and the roundtrip smoke (LLP 0265 T7)

[bgmcmullen]

What this is

LLP 0265 T7, the final task. It needs both T5 (#952) and T6 (#953), so this branch is T5 with T6 merged in; the PR is based on the T5 branch and the first few commits shown are T6's. Merge order: #951 (T4) -> #952 (T5) and #953 (T6) in either order -> this.

The three surfaces

• hyp query status reports index coverage. A summary line - grep index: N of M data files indexed (searches brute-scan the rest; compaction indexes them) - plus an indexed= extra on each searchable partition. cacheStatus computes it with a pure directory scan (indexedFileCount, grep dataset only), matching the cost profile of the other status counters. "Grep is slow on deep history" becomes diagnosable where the operator already looks, and the expected gap is explained in the line itself.
• The hypaware-query skill learns grep, in both host copies: a ## Full-text search section covering when to prefer it over LIKE '%...%' SQL, the ten-column coverage caveat (fixing the "core query subcommands are overview, schema, and sql" claim, and spelling out what zero hits does not prove), the truncation notice, the coverage/speed relationship with the status line to check, the sub-ngramLength literal cliff (a short literal defeats index pruning, never correctness), and local-only withholding parity with SQL. The read-class verb lists gain grep_search. The host-divergence fixture is re-recorded since the two deliberately host-specific lines both carry that list.
• A hermetic smoke, query_grep_roundtrip (added to the release battery in AGENTS.md), drives the real CLI dispatch end to end:
  1. scan-tier search before any index exists,
  2. hyp purge --session removing a row grep can then no longer surface,
  3. hyp query maintain --force compacting and building sidecars,
  4. the status coverage line reporting full coverage,
  5. the indexed tier answering identically - proved from query.grep_search span attributes (indexed_file_count>0, scanned_file_count=0), with a guard that no span carries the query text,
  6. LLP 0105 from three caller contexts: withheld with a count on stderr from the synced cwd, visible from the local-only cwd itself, restored by --include-local-only.

Everything the plan asked T7 to assert is asserted; the one variation is that the stale-sidecar purge case (sidecar built before the purge) lives in the T6 unit tests rather than the smoke, where the natural flow builds the sidecar after.

Verification

• npm run smoke -- query_grep_roundtrip: ok, first run.
• npm run typecheck: clean; unit suites for sidecar-build (now also pinning cacheStatus.indexedFileCount) and the verb: green.
• npm test: green modulo the two known environment-only failures noted on The local grep service: one newest-first file walk, sidecar-or-scan per file (LLP 0265 T4) #951.

With this, the plan's task graph is complete: T1-T3 merged, T4-T7 open as this stack. What remains outside the plan is server-repo work only (the import swap onto hypaware/core/search after the dependency bump).

🤖 Generated with Claude Code

neutral triage @ e45791b7

Verdict: zero residual findings; nothing left to defer. The one finding open after two review rounds (round 2 finding 1, the sidecarPathFor import that would break the tree when #953 merged in, tracked as #979) is applied at this head: maintenance.js:21-22 now imports sidecarPathFor from ../search/searchable_columns.js, which exports it, and #953's content (including the tool_args allowlist drop) is already in this branch's history. The branch was rebased since the previous triage, so all eleven findings from both rounds were re-verified against this tree file by file; all are landed. The maintainer's re-check items hold: the SKILL.md coverage bullet names exactly the nine post-#953 columns in both host copies, the verb coverage clause interpolates SEARCHABLE_COLUMNS, and the query_grep_roundtrip smoke references tool_args nowhere.

Verified on this head, per the unstick request: npm run typecheck clean; npm test 4650 pass, 0 fail; npm run smoke -- query_grep_roundtrip ok; CI green. The PR waits only on stack merge order (#951, then #952/#953 in either order, then this). #979 stays open until this PR merges, since the fix it tracks lives on this branch.

[philcunliffe]

neutral review - PR #954 @ 94a22b3a

Verdict: findings (6, all Low). No correctness bug found in the shipped surfaces. All six are fixed and pushed as d7322821.

Scope note: the PR diff against its base (grep/t5-query-grep-verb) also carries the T6 sidecar-build merge (e5be52a5). T6 has its own PR (#953), so this review is scoped to the T7 commit 94a22b3a; T6 files were read for context only.

What I verified beyond reading

• npm test: 4634 pass, 1 fail - test/core/hyparquet-floor-pin.test.js only, the known environment-only failure. No other failures.
• npm run typecheck: clean.
• npm run smoke -- query_grep_roundtrip: green. Also re-ran status_diagnostics, cache_lifecycle_maintenance, claude_telemetry_capture (the other flows that read cache status output) - all green, so the new grep index: line breaks no existing assertion.
• Checked and cleared a suspected bug in the coverage line. countDataFiles (src/core/cache/maintenance.js:1516) excludes only .index.parquet, so I expected Iceberg position-delete files in the same data/ dir to inflate the M in grep index: N of M and make the line permanently claim "compaction indexes them" on any purged partition. Probed it with an instrumented copy of the smoke: deletes are written as *-deletes.puffin, not .parquet, so countDataFiles never sees them. Observed after a purge: grep index: 0 of 1 with files=1 indexed=0 deletes=1; after maintain: grep index: 1 of 1. The line is accurate.
• Checked the sub-ngram claim in the skill doc. hypgrep/src/constants.js: defaultNgramLength = 5, so "a literal shorter than 5 characters defeats the index" is exactly right.
• Checked hyp query status is a real name. It is an alias of cache status (src/core/cli/core_commands.js:161-167), so the docs were not broken, only inconsistent (finding 6).

Findings

1. Low - the smoke's newest-first assertion is disjunctive and cannot fail.
hypaware-core/smoke/flows/query_grep_roundtrip.js:157 asserted sessions[0] === 'sess-purged' || sessions[0] === 'sess-new'. The seed dates make the answer exact (sess-new is 2026-08-03, sess-purged 2026-08-02, and the newer sess-private is withheld from this caller), so the either-or would pass through a one-day sort inversion in the very ordering the grep service promises. Fixed: pinned to sess-new; the smoke still passes, which confirms the tightened value.

2. Low - the smoke's header docstring names a CLI call the step does not make.
query_grep_roundtrip.js:46 said step 3 is hyp query maintain --force, but the step calls maintainCache() directly (line ~176) because it asserts on report.partitions[].sidecarsBuilt. In a file whose stated premise is "drives the REAL CLI dispatch -> verb -> grep service path", that is the one claim a reader would rely on. Fixed: the docstring now says what the code does and why.

3. Low - a smoke step whose span is guaranteed to be dropped.
query_grep_roundtrip.js:228-241: assert_telemetry was wrapped in step(), i.e. runRoot, after await obs.shutdown(). The harness contract (hypaware-core/smoke/lib/harness.js:24-26) is that shutdown is the flush, so that root span is opened against a shut-down provider and never reaches the trace. Every sibling flow that reads traces (join_flow_remote_config.js:261, local_parquet_export.js:271, otel_loopback_capture.js:93) reads them outside a step for exactly this reason. Against the repo's log-driven rule that every smoke step carries a smoke_step, a step that silently emits nothing is worse than no step. Fixed: unwrapped to a plain block, matching the siblings.

4. Low (reuse) - the sidecar naming contract is restated in the one place a drift would misreport.
src/core/cache/maintenance.js:1550 open-coded name.replace(/\.parquet$/, '.index.parquet'), and its own comment called this "the sidecar naming contract". That contract already has an owner, sidecarPathFor() in src/core/search/sidecar_build.js:75 - the function the build pass publishes under and the grep service probes with - and maintenance.js already imports from that module (buildSidecarsForTable). Two copies mean a future change to the suffix leaves the status line reporting coverage that does not exist. Fixed: imported and used sidecarPathFor.

5. Low - a test comment claims an assertion the test does not make.
test/core/search-sidecar-build.test.js:158-159 said indexedFileCount "stays absent elsewhere", but the fixture cache holds only ai_gateway_messages, so nothing in the test could distinguish "grep dataset only" from "every dataset". Fixed: the test now seeds a second (logs) partition and asserts indexedFileCount === undefined there, plus dataFileCount >= 1 so the equality assertion cannot pass on a pair of zeroes.

6. Low - the skill doc contradicts its own command routing.
hypaware-query/SKILL.md:45 (both host copies) told the reader to run hyp query status, two paragraphs after "Cache operations live under hyp cache" and a quick-reference block listing hyp cache status. The alias works, so nothing was broken, but an agent reading the file gets two names for one command with no signal which is canonical. Fixed: use hyp cache status, the registered name. Note the divergence fixture needs no re-record - test/helpers/skill_host_divergence.js hashes only the host-only lines, and this edit is identical on both sides.

Deferred

Nothing. All six are landed in d7322821, each verified by diffing the committed tree against 94a22b3a per file, not by the test suite alone.

Not findings, recorded so the next reader does not re-litigate them

• The grep index: summary line's label is wider than the cache:/pending:/datasets: column, so it does not align. Left alone deliberately: the exact wording is pinned by the smoke's regex and quoted in both skill copies, and "aligned" would cost clarity.
• indexed= on a partition line is rendered only in the source-table branch (src/core/commands/query.js:135), so a legacy epoch-layout grep partition contributes to the summary line but shows no per-partition extra. Correct for every partition the modern write path creates (appendRowsToSourceTable sets layout: 'source-table'), and legacy caches are migrated, so this is not worth a branch.

[philcunliffe]

neutral review - PR #954 @ d7322821 (round 2)

Verdict: findings (5). One Medium that will break the tree at integration and cannot be fixed on this branch alone, four Low/Trivial in the skill doc, all four fixed and pushed as 239ad4f6.

Scope: the T7 commits only (7d6fce5a..d7322821). The diff against the base also carries the T6 merge (e5be52a5), which is #953's; T4/T5 files were read for context but are #951/#952's to answer for. Round 1's six findings were re-checked at d7322821 and all six are correctly landed.

What I verified beyond reading

• npm test: 4634 pass, 1 fail, and the one failure is test/core/hyparquet-floor-pin.test.js, the known environment-only failure. Treating the suite as green.
• npm run typecheck: clean.
• npm run smoke -- query_grep_roundtrip: ok. Also re-ran status_diagnostics and cache_lifecycle_maintenance (the other flows that read query status output): both ok, so the new grep index: line breaks nothing.
• Probed the sub-ngram doc claim rather than trusting it. Built a one-row cache, ran a 3-character literal (cat) through executeGrepSearch on both tiers: scan tier 1 hit, then after buildSidecarsForTable, indexed tier 1 hit with scannedFiles=0. The doc's "the answer stays exact, only pruning is lost" is true; hypgrep degrades to a full read of the candidate file rather than returning nothing.
• Probed the --limit path, which produced finding 2 below: argvToParams accepts --limit 5000 (the schema carries no maximum), and the operation then substitutes DEFAULT_LIMIT for it.
• Checked the coverage counters cannot disagree. countDataFiles and the new countIndexedDataFiles both scan the same <live>/data directory with the same .parquet / not-.index.parquet rule, so indexed can never exceed files. In-flight builds are <sidecar>.tmp, outside both counts.
• Checked the smoke follows the house pattern for the command it drives. It calls ['query', 'status'], matching cache_lifecycle_maintenance.js:167 and claude_telemetry_capture.js:583; the doc's hyp cache status is the registered name and query status its alias (src/core/cli/core_commands.js:162). Both surfaces are internally consistent, so this is not a finding.
• CLAUDE.md is a symlink to AGENTS.md, so the one release-battery edit covers both.

Findings

1. Medium - maintenance.js imports sidecarPathFor from the module sibling #953 stops exporting it from. The merge is clean and the tree then fails to load.

src/core/cache/maintenance.js:21 is import { buildSidecarsForTable, sidecarPathFor } from '../search/sidecar_build.js'. #953's current head (07cd4661, "Review fixes on the sidecar build: one sidecar-path contract") moves that function to src/core/search/searchable_columns.js:63 and leaves no re-export behind: sidecar_build.js now imports it like everyone else.

Reproduced, not inferred. Merging 07cd4661 into this branch produces no conflict (different files, different lines), and then:

$ npm run typecheck
src/core/cache/maintenance.js(21,33): error TS2459: Module '"../search/sidecar_build.js"'
  declares 'sidecarPathFor' locally, but it is not exported.

$ node --test test/core/search-sidecar-build.test.js
SyntaxError: The requested module '../search/sidecar_build.js' does not provide an
  export named 'sidecarPathFor'

This is a module-load failure, not a runtime edge case, so it takes down every importer of maintenance.js: hyp cache status, hyp cache maintain, and the daemon's maintenance tick. It is loud rather than silent (typecheck and the suite both catch it), but it lands the instant the two branches meet.

The cause is symmetrical and slightly ironic: round 1's finding 4 removed the duplicated .index.parquet rule from maintenance.js by importing the owner, while #953's own review removed the same duplication by relocating the owner. Each fix is right; together they miss.

Verified remedy, left open. maintenance.js already imports GREP_DATASET from ../search/searchable_columns.js, so the whole fix is to move the symbol onto that existing line:

import { buildSidecarsForTable } from '../search/sidecar_build.js'
import { GREP_DATASET, sidecarPathFor } from '../search/searchable_columns.js'

I applied that on the merged tree and re-ran everything: typecheck clean, npm test 4634 pass / 1 known environment failure, npm run smoke -- query_grep_roundtrip ok.

Why this is left open rather than pushed: it cannot be applied to grep/t7-surfaces on its own, because searchable_columns.js does not export sidecarPathFor in this branch's tree yet. It belongs to whoever brings the two together: apply it as the merge resolution when #953 lands, or in #954's rebase onto #953. Flagging it for the orchestrator since merge order is not mine to drive.

2. Low - the doc names --limit as the remedy for truncation without its ceiling, and out-of-range values silently return fewer rows than the default suggests.

SKILL.md:41 said --limit <n> (default 50), and SKILL.md:44 told the reader to "raise --limit". But src/core/search/grep_verb.js:86-89 accepts the value only when it is an integer in 1..1000 and otherwise substitutes DEFAULT_LIMIT, and the input schema declares no maximum, so nothing refuses the flag first. Confirmed through the real codec: --limit 5000 parses to 5000 and the operation returns 50 hits, fewer than a caller who asked for more, with no diagnostic. An agent that reads the truncation notice and reaches for a big number gets the opposite of what it asked for. Fixed: the doc states the ceiling and the silent-fallback behavior, with the --limit 5000 case named.

Note the silent fallback itself lives in grep_verb.js, which is #952's file and outside this PR's diff, so I did not touch it. If it should refuse an out-of-range limit rather than quietly rewrite it, that belongs on #952.

3. Low - only one of the verb's two stderr completeness notices was documented, and they call for opposite responses.

grep_verb.js:126-133 emits grep: more matches exist beyond the limit ... when truncated, and grep: the search stopped before covering every file; results may be incomplete when exhausted === false (an abort or a server deadline mid-walk). SKILL.md:44 covered only the first, under the heading "Truncation is a notice, not an error". The second means hits can be missing from anywhere in the range and a wider limit will not recover them, which is the opposite remedy. In a doc whose stated job is teaching an agent what an empty or short answer does not prove, that omission is the same failure mode the coverage bullet exists to prevent. Fixed: both notices named and told apart.

4. Low - --remote described as running "the same search", in the same sentence as two flags that do not survive the trip.

SKILL.md:41 said "--remote <target> runs the same search against a server" immediately after introducing --regex, and the local-only bullet offers --include-local-only unqualified. The verb's own contract says otherwise: grep_verb.js:66 documents that servers restrict regex mode to the operator, and grep_verb.js:28-35 explains that include-local-only carries no schema default precisely because it goes over the wire and the server's grep_search schema rejects it ("the server enforces its own visibility, not the caller's"). Both exceptions were invisible to a reader of this section. Fixed: stated beside the flag.

5. Trivial - the new quick-reference line's trailing comment sat one column right of its neighbours.

SKILL.md:26 put its # at column 57 where lines 22 and 27 use column 56, so the block rendered ragged. Fixed: one space removed.

Findings 2 to 5 are identical edits in both host copies. All four touched lines are shared, not host-specific, so test/fixtures/skill-host-divergence.json needs no re-record; the divergence test passes unchanged, and diff of the two copies still shows exactly the two deliberate host-only lines.

Not findings, recorded so the next reader does not re-litigate them

• countIndexedDataFiles uses a bare readdirSync where countDataFiles uses withFileTypes and filters isFile(). A directory named x.parquet beside a file named x.index.parquet would count on one side and not the other. Theoretical only: nothing in the write path creates a directory with that suffix.
• The grep index: label is wider than the cache: / pending: column and does not align. Left alone in round 1 on purpose (the wording is pinned by the smoke's regex and quoted in both skill copies); still agree.
• The summary line sums dataFileCount across every partition carrying indexedFileCount, including a legacy epoch-layout one that renders no per-partition indexed= extra. Correct for everything the modern write path creates; already cleared in round 1.
• The @ref LLP 0264#lifecycle on the status line resolves (the anchor exists) and the gloss is defensible: that section is what makes "compaction indexes them; the rest is brute-scanned" true. LLP 0265's T7 bullet is arguably the closer anchor for "status gains a coverage line", but the existing ref is not dishonest, so I left it.

[philcunliffe]

neutral is stuck on PR #954 and needs a human decision

What neutral was doing. The triage rung on PR #954 (grep/t7-surfaces, head 239ad4f6), after the PR exhausted its 2-round review budget with a finding still open. Triage re-verified every finding from both rounds against the tree at head: round 1's six findings and round 2's findings 2-5 are all correctly landed (checked in the files, not just the prose). Exactly one finding remains open, and it is a blocker.

Why it cannot proceed. Round 2 finding 1: this branch and PR #953 each fixed the same duplication in opposite directions, and the tree breaks the instant they meet.

• src/core/cache/maintenance.js:21 (this branch) imports sidecarPathFor from ../search/sidecar_build.js (added by round 1's finding-4 fix, correct on this branch, where sidecar_build.js:77 exports it).
• PR Sidecar builds at maintenance: compaction finalizes a file, the index follows (LLP 0265 T6) #953's head (07cd4661, grep/t6-sidecar-build) moves sidecarPathFor to src/core/search/searchable_columns.js:63 and leaves no re-export behind; sidecar_build.js now imports it itself.

Triage reproduced the failure rather than inferring it: merging origin/grep/t6-sidecar-build into this branch's head completes with no conflict, and the merged tree then fails at module load:

SyntaxError: The requested module '../search/sidecar_build.js' does not provide
an export named 'sidecarPathFor'

That is a load failure of maintenance.js, so it takes down every importer: hyp cache status, hyp cache maintain, and the daemon's maintenance tick. It is loud (typecheck and the suite both catch it) but lands the moment both PRs are in one tree, and the git merge itself gives no warning.

It cannot be fixed on this branch alone: this branch's searchable_columns.js does not export sidecarPathFor yet, so switching the import here would break the branch by itself. Review round 2 verified the remedy on the merged tree (typecheck clean, suite green, query_grep_roundtrip ok):

import { buildSidecarsForTable } from '../search/sidecar_build.js'
import { GREP_DATASET, sidecarPathFor } from '../search/searchable_columns.js'

Non-blocking residuals: none. Every other finding from both rounds is verified landed at head 239ad4f6. The reviews also recorded a few deliberate non-findings (the unaligned grep index: label, the legacy epoch-layout partition rendering, the readdirSync variant in countIndexedDataFiles, the LLP 0264 anchor choice); those stand as recorded and need no action.

What neutral needs from you. A decision on where the one-line integration fix is applied, since it spans two open PRs and merge order is not triage's to drive. Options:

1. Land Sidecar builds at maintenance: compaction finalizes a file, the index follows (LLP 0265 T6) #953 first, then rebase or merge Sidecar builds at maintenance: compaction finalizes a file, the index follows (LLP 0265 T6) #953 into Surfaces and proof: status coverage, the skill learns grep, and the roundtrip smoke (LLP 0265 T7) #954's branch and apply the two-line import fix above as part of that update. (The fix is already verified on the merged tree.)
2. Apply the fix as the merge resolution when whichever PR lands second is merged.
3. Alternatively, have Sidecar builds at maintenance: compaction finalizes a file, the index follows (LLP 0265 T6) #953 keep a re-export of sidecarPathFor in sidecar_build.js; then Surfaces and proof: status coverage, the skill learns grep, and the roundtrip smoke (LLP 0265 T7) #954 needs no change. This trades away Sidecar builds at maintenance: compaction finalizes a file, the index follows (LLP 0265 T6) #953's "one contract owner" review fix, so options 1 or 2 are likely preferable.

How to unstick. Reply with a comment on this PR (or push to the branch); neutral monitors this thread and will re-engage with your guidance on its next tick.

[bgmcmullen]

Unstick: option 1 from your triage comment. #953 lands first.

Keep #953's move of sidecarPathFor into searchable_columns.js with no re-export - the single contract owner is the right end state, and option 3 trades it away for nothing.

So: #953 merges first (its tool_args blocker is answered on that thread - drop it from SEARCHABLE_COLUMNS, follow-up filed as #977). Then rebase or merge it into grep/t7-surfaces and apply the import fix as part of that update: maintenance.js:21 imports sidecarPathFor from ../search/searchable_columns.js instead of ../search/sidecar_build.js. You already verified that fix on the merged tree, so please re-run npm test, npm run typecheck, and npm run smoke -- query_grep_roundtrip on the rebased head to confirm the load failure is gone rather than trusting the earlier probe.

One thing to re-check while you are in there: #953's answer removes tool_args from the allowlist, so any T7 surface text that enumerates covered columns - the hypaware-query SKILL.md grep section and the verb's coverage clause - needs to match the new set, and the query_grep_roundtrip smoke must not assert a tool_args hit.

[philcunliffe]

Re-engaging with your guidance - option 1 taken: #953 lands first keeping the sidecarPathFor move with no re-export, then grep/t7-surfaces takes #953 and applies the maintenance.js:21 import fix from ../search/searchable_columns.js, re-running npm test, npm run typecheck and npm run smoke -- query_grep_roundtrip on the rebased head rather than trusting the earlier probe; also re-checking that the T7 surface text (the hypaware-query SKILL.md grep section, the verb coverage clause) and the query_grep_roundtrip smoke match the new column set now that tool_args is dropped.

[philcunliffe]

neutral triage - PR #954 @ 891f912b

Verdict: every residual finding is non-blocking. Deferred to #979; the PR waits only on #953 landing.

Re-verified both review rounds against the tree at head: round 1's six findings and round 2's findings 2-5 are all correctly landed. Exactly one finding remained open, round 2 finding 1 (the sidecarPathFor import that breaks the tree when #953's move merges in), and the maintainer's unstick decision on this thread settles both its resolution and its sequencing: #953 lands first with no re-export, then this branch takes #953 and moves the maintenance.js:21 import to ../search/searchable_columns.js, re-verifying on the rebased head. That makes it decided, tracked integration work rather than an open defect on this branch (whose own tree typechecks clean and passes the suite), so it is deferred to #979, not a blocker. It could not be applied this tick: #953's head was still moving, and the import switch before #953 is in this tree would break the branch by itself.

The maintainer's "re-check while you are in there" item was independent of #953 landing, so it is done now, pushed as 891f912b:

• hypaware-query SKILL.md, both host copies (one shared line): the coverage bullet now states nine columns with tool_args moved to the not-searched list (system_text, tools, tool_args, attributes, raw_frame; read those with SQL). The two deliberate host-only lines are untouched, so the divergence fixture needed no re-record (skill-host-parity passes unchanged).
• The verb's coverage clause needs no edit: grep_verb.js:49 interpolates [...SEARCHABLE_COLUMNS].join(', '), so it follows Sidecar builds at maintenance: compaction finalizes a file, the index follows (LLP 0265 T6) #953's drop automatically; a hand edit here would only drift.
• The query_grep_roundtrip smoke asserts no tool_args hit: its seeded schema carries no tool_args column at all and the needle lands in content_text only. Verified, nothing to change.

Verification at 891f912b: npm run typecheck clean; npm test 4634 pass with the one known environment-only failure (hyparquet-floor-pin); npm run smoke -- query_grep_roundtrip ok.

[philcunliffe]

neutral triage - PR #954 @ e45791b7 (re-triage after the deferred fix landed)

Verdict: zero residual findings. No new follow-up issue is needed: the only open finding from either review round was round 2 finding 1, the item #979 tracks, and it is now genuinely applied at this head. src/core/cache/maintenance.js:21-22 imports buildSidecarsForTable from ../search/sidecar_build.js and sidecarPathFor from ../search/searchable_columns.js, which exports it at line 70; sidecar_build.js no longer exports it and imports it like everyone else. #953's content is in this branch's history (c06017a3, and c00072b1 carries the tool_args allowlist drop), so the exact integration shape the maintainer chose (option 1) is what this tree contains.

The branch was rebased since the previous triage (891f912b is not an ancestor of e45791b7), so every finding from both rounds was re-verified against this tree, not trusted from prose. All eleven are landed. The maintainer's re-check items also hold at this head: the SKILL.md coverage bullet lists exactly the nine post-#953 columns (matching SEARCHABLE_COLUMNS, both host copies, only the two deliberate host-only lines diverge), the verb coverage clause interpolates SEARCHABLE_COLUMNS so it followed the drop automatically, and the query_grep_roundtrip smoke references tool_args nowhere.

Verification on the rebased head, as requested, rather than trusting the earlier merged-tree probe:

• npm run typecheck: clean.
• npm test: 4650 pass, 0 fail, 1 skip (in this fresh worktree even the known environment-flaky hyparquet-floor-pin passed).
• npm run smoke -- query_grep_roundtrip: ok.
• CI on this head: all checks green.

What the PR waits on: nothing on this branch. The stack ahead of it: #951 (T4) merges first, then #952 (T5) and #953 (T6) in either order, then this. #979 stays open until this PR merges, since the fix it tracks lives on this branch, not on a merged base; whoever merges #954 can close it.

[bgmcmullen]

Superseded by #984, which carries this work (and every review fix from this PR) on a single branch cut from current master.

The stack could not be merged in sequence: this repo squash-merges, so merging one PR put a commit on master sharing no history with the branches behind it, and each remaining PR then re-proposed its parent's changes. Verified by simulation: merging #951 and then #953 produced add/add conflicts in grep_service.js and search-grep-service.test.js.

Integrating on current master also surfaced two problems invisible here: a sidecarPathFor import left dangling when #953's review moved the function, and master's new D1 short-flag gate (LLP 0293), which query grep trips as the second verb with a greedy positional. Both are fixed in #984.

The branch is retained, so this can be reopened if needed.

This PR's task: T7, surfaces and the roundtrip smoke.