From c7004334f424347c5a8e193e73d3e91a1b03abdf Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Tue, 6 Oct 2026 14:34:24 +0300 Subject: [PATCH 1/8] docs: record search and grep header footer follow-up Capture the user-selected presentation follow-up after PR 454, with observed output differences, formatter ownership and acceptance criteria. Keep it separate from routing guidance and defer implementation until the concrete output shape is selected. --- docs/plans/open-backlog.md | 43 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/docs/plans/open-backlog.md b/docs/plans/open-backlog.md index f6ba7235..0697ebbf 100644 --- a/docs/plans/open-backlog.md +++ b/docs/plans/open-backlog.md @@ -1,5 +1,48 @@ # Open backlog +## Unify search and grep headers and footers + +User selected this as a follow-up after merging +[PR #454](https://github.com/githits-com/githits-cli/pull/454) on 2026-10-06. +The merged Sources/Preparing boundary remains the foundation. This is a bounded +presentation increment, separate from tool routing and investigation guidance. + +Evidence: authenticated dev CLI and local MCP captures returned usable Express +documentation while repository code indexed. Search's header was +`1 partial result | 1 docs page | indexing | 0/1 ready | next_offset=1`; +grep's was `1 match in 1 line across 1 page; more available`. +Search's unqualified readiness count is ambiguous beside usable docs results, +and pagination notation competes with the outcome. Footer wording and anatomy +also differ (`Next: use these hits now` versus `# Read pages` and +`More matches: repeat this grep, adding:`); grep's long opaque cursor dominates +its continuation. Indexed-alternative summaries use both `+7` and `(+5 more)`. +The permanent capture context is in +[search-snapshot-presentation.md](../implementation/search-snapshot-presentation.md#shared-sourcepreparation-boundary-2026-10-06). + +Outcome: give search and grep a common outcome-first header and footer anatomy: +returned evidence, Sources/Preparing, results, then read, pagination and optional +wait/retry guidance. Retain each tool's meaningful counts and semantics rather +than forcing identical result labels. Make readiness scope explicit and evaluate +moving pagination mechanics out of the outcome headline. Reuse common wording +where the facts and actions are equivalent, including alternative-summary +notation when using shared preparation copy. + +Ownership: the existing shared CLI/MCP tool formatters own placement and native +actions; shared presentation helpers own genuinely repeated wording. Establish +the concrete output shape before deciding whether another helper is warranted. +Core services do not own display copy. No backend/schema work, new output mode, +flag, layout framework or change to JSON is implied. + +Acceptance: compare actual CLI and MCP search/grep output for ready results, +ready docs with pending code, multiple targets with mixed readiness, empty +results and continuation pages. A reader must distinguish usable results from +preparing scopes without decoding `0/1 ready`. Both tools should order read, +more-results and wait/retry actions consistently, with exact backend operands, +opaque cursors and existing wait units preserved. Investigate a less intrusive +cursor layout without truncating it or inventing a replacement. Verify narrow +and normal widths, ANSI-free parity, truthful coverage and retained indexing +alternatives. Broader read/list output changes require separately verified scope. + ## Optimize overall tool routing and investigation instructions User deferred this follow-up until after unified MCP grep adoption on From 01994dfb1f73f980a05afc86d08e84d818d3135f Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Tue, 6 Oct 2026 23:24:32 +0300 Subject: [PATCH 2/8] docs: design shared search and grep headers and footers Specify outcome-first headers, truthful partial and pagination qualifiers, and a shared Read / More results / Follow-up footer. Record verified service constraints, review closures and focused implementation acceptance without changing production behavior or opening a planning-only PR. --- docs/plans/open-backlog.md | 42 +- docs/plans/search-grep-headers-footers.md | 443 ++++++++++++++++++++++ 2 files changed, 445 insertions(+), 40 deletions(-) create mode 100644 docs/plans/search-grep-headers-footers.md diff --git a/docs/plans/open-backlog.md b/docs/plans/open-backlog.md index 0697ebbf..13d3f865 100644 --- a/docs/plans/open-backlog.md +++ b/docs/plans/open-backlog.md @@ -2,46 +2,8 @@ ## Unify search and grep headers and footers -User selected this as a follow-up after merging -[PR #454](https://github.com/githits-com/githits-cli/pull/454) on 2026-10-06. -The merged Sources/Preparing boundary remains the foundation. This is a bounded -presentation increment, separate from tool routing and investigation guidance. - -Evidence: authenticated dev CLI and local MCP captures returned usable Express -documentation while repository code indexed. Search's header was -`1 partial result | 1 docs page | indexing | 0/1 ready | next_offset=1`; -grep's was `1 match in 1 line across 1 page; more available`. -Search's unqualified readiness count is ambiguous beside usable docs results, -and pagination notation competes with the outcome. Footer wording and anatomy -also differ (`Next: use these hits now` versus `# Read pages` and -`More matches: repeat this grep, adding:`); grep's long opaque cursor dominates -its continuation. Indexed-alternative summaries use both `+7` and `(+5 more)`. -The permanent capture context is in -[search-snapshot-presentation.md](../implementation/search-snapshot-presentation.md#shared-sourcepreparation-boundary-2026-10-06). - -Outcome: give search and grep a common outcome-first header and footer anatomy: -returned evidence, Sources/Preparing, results, then read, pagination and optional -wait/retry guidance. Retain each tool's meaningful counts and semantics rather -than forcing identical result labels. Make readiness scope explicit and evaluate -moving pagination mechanics out of the outcome headline. Reuse common wording -where the facts and actions are equivalent, including alternative-summary -notation when using shared preparation copy. - -Ownership: the existing shared CLI/MCP tool formatters own placement and native -actions; shared presentation helpers own genuinely repeated wording. Establish -the concrete output shape before deciding whether another helper is warranted. -Core services do not own display copy. No backend/schema work, new output mode, -flag, layout framework or change to JSON is implied. - -Acceptance: compare actual CLI and MCP search/grep output for ready results, -ready docs with pending code, multiple targets with mixed readiness, empty -results and continuation pages. A reader must distinguish usable results from -preparing scopes without decoding `0/1 ready`. Both tools should order read, -more-results and wait/retry actions consistently, with exact backend operands, -opaque cursors and existing wait units preserved. Investigate a less intrusive -cursor layout without truncating it or inventing a replacement. Verify narrow -and normal widths, ANSI-free parity, truthful coverage and retained indexing -alternatives. Broader read/list output changes require separately verified scope. +Selected for design on 2026-10-06. The concrete design and implementation +acceptance criteria now live in [search-grep-headers-footers.md](search-grep-headers-footers.md). ## Optimize overall tool routing and investigation instructions diff --git a/docs/plans/search-grep-headers-footers.md b/docs/plans/search-grep-headers-footers.md new file mode 100644 index 00000000..554213af --- /dev/null +++ b/docs/plans/search-grep-headers-footers.md @@ -0,0 +1,443 @@ +# Search and grep headers and footers + +## Status and destination + +- Overall: **READY FOR IMPLEMENTATION**. +- Phase 1: **READY** — one implementation increment makes search/status and + grep headers and footers consistent on CLI and local/published MCP package text. +- Product decisions: **none open**. This document selects a concrete presentation + for the user-requested follow-up. The user can revise the examples before coding. +- Dependencies: merged PR #454 (`c71ffb5`), current main `bc295b3`, existing source/preparation facts, + existing read actions, search offset and grep cursor contracts. + +Readers should immediately see what this page returned, what is preparing, and +how to read, continue or obtain updated results. Search and grep retain their different evidence +counts and continuation semantics within one visible anatomy. This is one +bounded PR with product changes, not a planning-only PR. No production edits +are authorized by this planning turn. + +## Verified baseline + +Authenticated dev captures on 2026-10-06 exercised all four commands with CLI +`npm:express@2.3.10` and local MCP `npm:express@2.3.11`. Both were registry-confirmed +and unindexed during the calls. The durable record is +[search-snapshot-presentation.md](../implementation/search-snapshot-presentation.md#shared-sourcepreparation-boundary-2026-10-06). +Final full text and metadata are preserved locally in +`/tmp/shared-source-final-outputs.md` and `/tmp/shared-source-final-metadata.md`; +these files are supplemental evidence, not implementation dependencies. + +Search returned a usable documentation result but started with: + +```text +1 partial result | 1 docs page | indexing | 0/1 ready | next_offset=1 +``` + +Grep returned usable documentation while the same repository was preparing: + +```text +1 match in 1 line across 1 page; more available +``` + +Both already display Sources and Preparing. Search includes requested aliases +on its hosted-doc source when needed; grep scopes and source attribution have +separate semantics. Those facts must survive; equal-looking strings are not +proof of equal facts. + +Main was refreshed during planning to `bc295b3` (release0.27.0 and an +example-source tool). The diff from `c71ffb5` changes none of the inspected +search/grep formatters, projection/response, status API or smoke validators. +These selected contracts therefore remain current; implementation starts from +then-current main rather than requiring this planning branch as a code base. + +Code inspection on the merged baseline established: + +- `unified-search-text.ts` owns the outcome and action rendering; the status + renderer delegates to it. `UnifiedSearchTextResult.nextOffset` is already + available for both initial and retained-status pages. +- `progress.targetsReady/targetsTotal` is target readiness, not a count of usable + hits or ready documentation contributors. Remove this ambiguous text shorthand; + JSON retains the counts and per-scope context retains readiness facts. +- Search currently puts an offset only in the headline; it has no dedicated + pagination footer. Its lifecycle action union describes polling/retry, not + pagination. A healthy completed result has action `none`, so it also omits the + existing first-hit read example. +- `search` accepts `--offset` / `offset`; `search-status` / `search_status` does + not accept an offset. Status query echo omits original compile/filter options. + Never reconstruct a complete filtered search from a status result or hit label. +- Grep's `totalMatches` is occurrences on this page. The formatter derives unique + matching lines and files/pages. `nextCursor` is a complete backend operand; + existing validation rejects resumable traversal without a cursor. +- Grep `hasCoverageGap` treats any traversal other than COMPLETE as a gap. + `RESUMABLE_LIMIT` is normal pagination, so this predicate alone cannot drive a + new incomplete-coverage headline. Keep exhaustive/no-match decisions intact. +- Search availability distinguishes backend partial results from active interim + snapshots. Completed partial responses also exist; completion must not erase + that fact. Terminal and unknown lifecycle states remain independently visible. +- Footer read operands come from `readTarget` or grep's existing native templates. + `indexing-wait.ts` and `discovery-indexing-wait.ts` retain native wait policy. +- Search's bounded alternative suffix is ` +N`; read recovery already uses + `(+N more)`. Category names such as versions versus versions/refs reflect + different facts and must not be normalized into false equivalence. + +No performance claim or changed computation path is proposed. A benchmark is +not needed for choosing these text layouts; no optimization is part of scope. + +## Selected output design + +### Header + +Use the same ASCII ` | ` separator for outcome, tool-specific counts and short +qualifiers. Keep the headline free of request parameter names and readiness +fractions. Preserve pluralization and each tool's definition of its counts. + +```text +1 result | 1 docs page | partial | more available +1 match in 1 line across 1 page | more available +3 results | 2 repo code hits, 1 docs page +4 matches in 3 lines across 2 files | coverage incomplete +``` + +Search `partial` follows backend partialResults for active and completed pages. +An active, non-partial snapshot with hits uses `interim` instead. `more available` +follows hasMore / nextCursor and never implies exhaustive coverage. +For grep, classify gaps from the existing facts before choosing a headline: + +- Actual coverage gaps: a scope has readiness other than CURRENT, excluding + UNSPECIFIED paired with RESUMABLE_LIMIT (documented unvisited pagination); + traversal other than COMPLETE/RESUMABLE_LIMIT, an error or recorded scan omissions/issues, + or overall traversal is NON_RESUMABLE_PARTIAL, FAILED or CURSOR_EXPIRED. +- Retryable omissions only: at least one unavailableTarget, every omission is + retryable, and no actual coverage gap above. This includes preparing work and + existing retryable non-preparing reasons. Use the pagination exception in the + formatter-local omissions-only classification. +- Non-retryable omissions: any unavailableTarget with retryable=false. + +Hit-bearing pages use `partial` for retryable omissions only, and +`coverage incomplete` for any actual coverage gap or non-retryable omission +(the stronger qualification wins, so never print both). Append `more available` +independently for a supplied nextCursor. Both CURRENT+RESUMABLE_LIMIT and +UNSPECIFIED+RESUMABLE_LIMIT with no error/scan issues are ordinary pagination +and add neither partial nor incomplete-coverage copy. Apply this exemption +consistently to headline, omissions-only and zero-page decisions. Preserve the +unvisited source row and its existing no-results-on-this-page qualifier; do not +pretend that unvisited content was searched. +Keep the existing exhaustive predicate for claiming a full no-match search. +Do not change service validation or source-evidence projection. + +Search prints lifecycle separately immediately below its headline when active, +terminal or unknown; completed search needs no lifecycle line: + +```text +1 result | 1 docs page | partial | more available +Search: indexing +``` + +This preserves INDEXING/SEARCHING/PENDING/deferred/timeout/failed/unknown distinctions +without presenting target counts as result readiness. Grep has no persistent +search session and does not invent this line. Existing grep cursor-expiry and +scope coverage explanations remain before matches. + +Zero/no-snapshot outcomes keep their precise meanings: + +- Completed search with no hits: `No results`; when hasMore is true: + `No results on this page | more available`. Add `partial` before the pagination + clause when the actual snapshot has partialResults=true. +- Active search with an empty snapshot: `No results yet`; absent snapshot: + `No result snapshot yet`, each followed by its Search lifecycle line. Empty + snapshots append `| partial` when true; absent snapshots cannot claim partial. +- Terminal/unknown search keeps `No results` versus `No result snapshot`, followed + by its explicit lifecycle line and existing recovery disposition. Retained + empty snapshots keep the same partial qualifier when supplied. + +Grep's zero-hit headlines use these exact cases (pagination is independent): + +| Returned page | Outcome | +| --- | --- | +| Exhaustive, no omissions, no cursor | `No matches.` | +| Only retryable omissions, no cursor | `No matches yet.` | +| No other coverage gap or omission, valid cursor | `No matches on this page | more available` | +| Only retryable omissions, valid cursor | `No matches yet on this page | more available` | +| Scope/scan/traversal failure or any non-retryable omission, no cursor | `Zero returned matches; coverage is incomplete.` | +| Same incomplete case, valid cursor | `Zero returned matches | coverage incomplete | more available` | + +Retryable omissions include preparing repository/docs work as well as existing +retryable non-preparing reasons. Preparing is not called a failure. Sources, +Preparing and Omitted rows retain the exact reason and target attribution. +The captured hit-bearing Express docs/pending-code example therefore becomes +`1 match in 1 line across 1 page | partial | more available`; +its zero-hit equivalent follows the fourth row, without losing either "yet" or +the available continuation. No supplied cursor is silently hidden. + +### Body and source sections + +Order stays outcome/lifecycle, Sources, Preparing, scope warnings/recovery, +then tool-native hits/matches. Do not move actionable per-target remediation +into an unattributed global footer. Preserve source ordering, requested aliases, +zero-hit source disclosure, actual job identity, dates and all coverage facts. + +### Footer + +Use three optional sections, in this fixed order, separated by one blank line: + +1. **Read:** existing concrete example or file/page templates. +2. **More results:** repeat the original request with its exact continuation. +3. **Follow-up:** existing optional polling, omitted-target retry, fresh-search or + query-rewrite advice, with its conditions retained. + +If the existing search action has useResults=true but no returned read action +exists, omit Read and retain `Use these results now.` as the first plain-prose +line in Follow-up, before its conditional wait/fresh-search advice. More results +remains earlier in the footer. Never invent a read locator. + +No section is printed without a real action or meaningful advisory. Header and +labels use identical wording on CLI and MCP; action syntax remains native. + +Search selects the first actual returned read action as an example even on a +healthy completed page. Lead with `Use these results now; example read:` rather than +implying it reads every result. Grep retains file/page templates, removes +only their leading `#` and prefixes CLI templates with `githits` so they are +consistent command recipes; no template appears for an empty page. Example: + +```text +Read: + Use these results now; example read: + githits read 'https://expressjs.com/llms/resources.txt' --selector 'route' + +More results: + Repeat the original search, adding: + --offset 1 + Results may change while this search is running. + +Follow-up: + If you need updated results, wait (hits and order may change): + githits search-status --wait 80 +``` + +```text +Read: + Pages: githits read --lines $start-$end -- $url + +More results: + Repeat the original grep, adding: + --cursor '' + +Follow-up: + To retry omitted targets, rerun the original query with --wait 80000. +``` + +Examples above are designed layouts using captured facts, not new live renders; +placeholder locators are explicitly illustrative. MCP uses the same labels with +`read target=...`, `offset=1`, `cursor=...`, `search_status search_ref=...` and +`wait_timeout_ms=...`. Search CLI wait remains seconds; grep wait remains ms. +The implementation must preserve the existing exact native read arguments instead +of deriving them from display labels; the grep CLI prefix changes presentation +only, not the command or operands. + +For active/interim search pagination, add under More results: +`Results may change while this search is running.` Repeating search is not +continuing an immutable snapshot. Retained terminal results instead preserve +their existing mutable-evidence warning and fresh-search requirement; an ended +search reference never becomes pollable. More results always says *original +search*, including on search-status output, because its query echo cannot +reconstruct caller filters/targets. hasMore with no nextOffset remains a truthful +`More results are available; repeat the original search.` advisory without +inventing an offset. Never compute it from the visible hit count. + +Prior-HEAD specific-ref guidance stays attached to the search read/follow-up advice. +Keep `If you need current HEAD` when that proof exists, the hits/order warning, +query-rewrite choices and no-poll terminal rules. Known terminal statuses are +DEFERRED/TIMEOUT/FAILED; other status values retain existing `status unknown` +wording rather than claiming a new backend lifecycle contract. Empty results have no Read +section; pagination and recovery are independently optional, not gated by the +lifecycle action union. + +The full cursor cannot become shorter within the existing contract. Keep it +unwrapped and exact, dim action lines on ANSI CLI in both tools, and place each on its own +indented line. Section labels are bold, prose is plain, and all footer action +lines (read, offset/cursor and status commands) are dim. Use a tiny shared +action-line styling function; per-tool renderers pass exact action strings, so +the helper never guesses whether prose is a command. ANSI-free output carries +identical words, operands and order. It will still take space; truncation, +local handles, files, clipboard integration or a new cursor API are out of scope. + +Search alternative summaries use `(+N more)` rather than `+N`. Preserve existing +limits, ordering, version/ref categories and suggested-versus-indexed meaning. +Uncounted `+more` evidence retains its unknown-count meaning; no synthetic count. +Read/list/resolve output and their alternatives do not otherwise change in this +PR. Document this intentional scope boundary so their older footer labels are +not mistaken for accidental drift. + +## Architecture and scope + +Shared MCP presentation naturally owns repeated output copy because CLI and MCP +already call these neutral formatters. Per-tool adapters own counts, lifecycle, +coverage and native actions; core services continue owning data and validation. +A Commander-level helper would duplicate MCP behavior; a core helper would put +presentation in transport. Neither is appropriate. + +Use one small pure `packages/mcp/src/shared/search-grep-output-text.ts` helper for +` | ` headline joining/wrapping/emphasis and the fixed Read/More results/Follow-up +section skeleton. Its inputs are already-rendered headline clauses and optional +read/more/follow-up lines; it knows no backend state, target identity or command +arguments. Treat action lines as verbatim strings; only formatter-authored prose +is wrapped before supplying it. No configurable section registry, formatter DSL, +state machine, service DTO, runtime dependency or public export is needed. + +Tool formatters retain all decisions and use this helper. Footer pagination is +rendered separately from search's existing lifecycle action; there is no reason +to add pagination to that semantic union. Reuse `renderReadTarget`, exact quoting, +existing wrapping/colors and wait policy. Header wrapping must work on both +surfaces; current search's unsplit first line needs the same width treatment as +existing grep prose. Do not rewrap returned source content or action operands. + +Likely production files: `unified-search-text.ts`, `unified-search-status-text.ts` +(only if passing existing result facts needs adjustment), `grep-text.ts` and the +new small helper, plus affected structural assertions in +`scripts/cli-smoke.ts` and `packages/mcp/src/smoke-test.ts`. These validators +currently recognize `Next:` and old grep read labels; migrate only search/grep +assertions, leaving unrelated resolve/list checks intact. The existing +`UnifiedSearchAvailability` private projection gains the supplied partialResults +boolean (false when no snapshot), so empty snapshots can retain partial truth. +That fact naturally belongs in availability, not duplicated in CLI adapters or +inferred from hit count; add a focused projection test. No response/service/request/schema/descriptor changes +are planned. JSON and API field selections stay byte/structurally equivalent. +Shared header/footer placement replaces duplication without reworking result +bodies or source/preparation ownership. Existing mapped errors retain their +contracts; this change concerns successful/retained result text, not auth errors. + +## Phase 1 — consistent and truthful result edges + +- Status: **READY**. +- Expected outcome: the examples above hold for CLI/MCP search/status and grep; + usable results, pending scopes and available actions are immediately clear. +- Assumptions: existing nextOffset/cursor/read/lifecycle facts suffice (verified + above); fixed three-section helper needs no new service data; long cursors + remain unavoidable within the existing contract. +- Unknowns/product decisions: **none**. Implementation evidence may expose a + contradiction; report it before widening the scope or changing the design. +- Dependencies: reviewed plan, merged source rows and current main baseline. + +Ordered implementation: + +1. Add behavioral fixtures for headers and independently optional footer actions + in existing formatter tests. Add the small shared helper and integrate search + outcome/lifecycle lines and grep separator/coverage clauses. +2. Separate search read/pagination/follow-up rendering while preserving its semantic + action projection; integrate grep's existing actions through the same skeleton. + Preserve exact locators/cursors/wait units and all target recovery/body output. +3. Normalize search's counted alternative suffix. Scan affected structural smoke + assertions and parity tests for old header/footer assumptions; update only + expectations covered by this contract. +4. Update `search-snapshot-presentation.md`, `unified-grep.md`, + `mcp-cli-parity.md` and relevant output examples in `cli-commands.md`/`tools.md`. + Add an independent changes fragment with pending **patch** impact for both + githits and @githits/mcp (text-only behavior). No version/changelog edits. +5. Focused verification, internal review and a fresh Claude implementation review + loop; commit/push and one draft implementation PR. No merge or release. + +Acceptance cases: + +- Ready search code/docs/mixed hits and ready grep multiple occurrences on one + line: counts are correct, JSON unchanged, same separators/section labels. +- Ready docs plus pending code, including multiple targets and duplicate aliases: + Sources/Preparing stay truthful; no ambiguous readiness fraction; search and + grep both say partial for usable docs while code prepares; grep normal + pagination alone never claims incomplete coverage. +- Active non-partial interim and completed partial search: retain interim/partial + truth independently of completed state. PENDING/INDEXING/SEARCHING and + DEFERRED/TIMEOUT/FAILED/unknown remain visible and receive only their valid actions. +- Empty complete, empty active, absent snapshot, zero-hit continuation pages, + withheld scopes, actual coverage issues and expired grep cursor: precise + outcomes, no spurious Read, unchanged scope remediation. +- Search/status nextOffset and hasMore-without-offset, active mutable ordering, + terminal retained pages and grep cursors: More results is independent of Follow-up, + original-request controls retained, no status offset or invented operand. +- Multi-target grep with an unvisited UNSPECIFIED+RESUMABLE_LIMIT scope: + both hit-bearing and zero-hit continuation pages omit false coverage-incomplete + copy, keep the source page qualifier and exact cursor, with no invented retry. +- Grep zero-hit docs continuation with a retryable pending repository, and the + same shape with non-retryable omissions: exact table outcomes, correct + coverage qualifier, retained cursor and attributed omission reasons. +- Active search with hits: explicit `Use these results now` before + `If you need updated results, wait`; no required retry implied for usable hits. + Readless active hits retain use-now as the first Follow-up line, omit Read and + never invent a read target. +- Prior HEAD, current HEAD and ended evidence: read-now before optional wait, + exact specific-ref advice, no pollable ended search. +- 40/80/120-column output, ANSI stripped versus plain output and backend Unicode: + prose wraps, fixed actions/content do not; all footer sections omit cleanly. + +Verification: use bun test for the shared helper and existing +`unified-search-text.test.ts`, `unified-search-status-text.test.ts`, +`unified-search-snapshot-text.test.ts`, `unified-search-presentation.test.ts`, +`grep-text.test.ts`, `grep-text-rendering.test.ts`, `grep-response.test.ts`, +`indexing-estimates.test.ts`, `unified-search-semantic-text.test.ts`, and root +`search-parity.test.ts`/`grep-parity.test.ts`. Run affected smoke-assertion unit +cases, typecheck and both builds. Run `bun run smoke:cli`, `bun run smoke:mcp`, +`bun run smoke:cli:built` and `bun run smoke:mcp:built`; built checks verify +changed structural smoke assertions against packed Node launch paths. Run public +package validation after builds. Broader unit suite is the final integration +check after formatter changes; do not repeat it after wording-only closure. + +For live verification, use authenticated dev only, remove unintended endpoint +and token overrides without displaying credentials, and compare search/grep +on an indexed pinned package and a registry-confirmed unindexed package. +Use `route` search and literal `foo` grep for Express documentation, limit 1, +and the same pinned input within each client; if the version becomes indexed, +record the transition and verify pending cases against a different verified +unindexed version rather than inventing its version number. Use +literal CLI --wait 1 and matching MCP waits, record actual input/version and +per-response states. Use fixtures for terminal/unknown/expired states rather +than waiting for sessions to expire. Inspect captured output manually as well as +structural checks. Agent-facing text behavior also requires targeted +`bun run agent:e2e` search-investigation and grep-mixed-docs workloads selected +from eval/agentic/README.md: +`GITHITS_ENV=dev bun run agent:e2e --agent claude --surface mcp --server local --workload eval/agentic/workloads/unified-search-investigation.md` +and the same command with +`--workload eval/agentic/workloads/grep-mixed-docs.md`. +Inspect actual calls/final answer/isolation/metrics. +Previous Claude runs failed provider login before tool use, which is no quality +result; verify current availability and report the same limitation if it persists. +No descriptor/instruction changes or broad routing eval is implied. + +No new auth, deployment, performance or network risk is introduced. Programmatic +callers retain JSON; users receive revised text-v1 prose. Existing partial +search opt-out, wait defaults, selectors, filters, ordering and page size remain. +Hosted MCP adoption still follows its package release/dependency/deployment path. + +## Review, completion and cleanup + +Plan review: internal technical review followed by Claude Opus 5.5 external +rounds until clean (maximum three); adjudicate findings against the selected scope and verified facts. +Implementation review is a separate fresh loop. Keep the plan through its clean +round, move final facts/examples to permanent implementation docs, then delete +this plan in the final implementation PR commit. No separate cleanup PR. +Remove the selected backlog entry now that this plan owns it, replacing it with +one link while pending; delete that link on completion. The broader +`search-output-ux.md` plan's unrelated per-tool migrations are not absorbed; +this user-selected two-tool follow-up is an explicit narrow cross-tool increment. +One phase means no intermediate merge/reorientation boundary. If verified evidence +requires another phase or a broader design, stop and replan with the user. + +Internal technical review is **clean** after correcting empty partial-search +provenance, empty resumable grep wording and the known terminal status list. +The private availability correction is the smallest boundary change needed; +public JSON and service contracts stay unchanged. External Claude round 1 accepted direction and found five plan corrections: +read-now/conditional wait wording, omitted-target pagination semantics, action +styling, grep CLI read prefix and missing focused test files. All are accepted +and applied. Round 2 accepted four closures and refined the remaining grep +wording: retryable omissions only now say partial, while actual gaps and +non-retryable omissions say coverage incomplete. The readless advisory has an +explicit Follow-up slot; the read/follow-up wording is corrected. Round 3 found one remaining documented unvisited-scope case: +UNSPECIFIED+RESUMABLE_LIMIT is ordinary pagination, now exempted consistently +in header/omissions-only/zero-page rules and acceptance. The three-round cap is +reached; no fourth external round is dispatched. Coordinator closure verifies +this against implementation documentation and its existing fixture tests. +Final internal technical closure is clean. Existing unvisited-scope contract +proof passed: `bun test packages/mcp/src/shared/grep-text.test.ts +packages/mcp/src/shared/grep-response.test.ts --test-name-pattern 'lists unvisited +scopes beside the matched source|retains an unvisited selected site'` — 2 passed, +0 failed. These tests verify the baseline contract, not the proposed new output. +All findings are corrected; no direction, scope or product question remains. +There was no finding-free external round within the cap; implementation will +receive its own fresh review loop and actual output verification. No production change or planning-only PR created. From 5c3f1fd1588539725db45e10a6408d2713211842 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Tue, 6 Oct 2026 23:33:30 +0300 Subject: [PATCH 3/8] docs: revise result headlines toward plain language Replace the contested pipe-separated counters and unexplained partial labels with proposed outcome sentences and attributed limitation explanations. Mark the headline design under discussion and keep prior review evidence explicitly historical. --- docs/plans/search-grep-headers-footers.md | 190 ++++++++++++---------- 1 file changed, 105 insertions(+), 85 deletions(-) diff --git a/docs/plans/search-grep-headers-footers.md b/docs/plans/search-grep-headers-footers.md index 554213af..2b6953cd 100644 --- a/docs/plans/search-grep-headers-footers.md +++ b/docs/plans/search-grep-headers-footers.md @@ -2,11 +2,12 @@ ## Status and destination -- Overall: **READY FOR IMPLEMENTATION**. -- Phase 1: **READY** — one implementation increment makes search/status and +- Overall: **DESIGN REVISION — headline wording under discussion**. +- Phase 1: **DESIGN REVISION** — one implementation increment makes search/status and grep headers and footers consistent on CLI and local/published MCP package text. -- Product decisions: **none open**. This document selects a concrete presentation - for the user-requested follow-up. The user can revise the examples before coding. +- Product decisions: settle the revised plain-language headline examples. On + 2026-10-06 the user rejected the pipe-separated counters and unexplained partial + label as ambiguous, including for agents. The earlier readiness is superseded. - Dependencies: merged PR #454 (`c71ffb5`), current main `bc295b3`, existing source/preparation facts, existing read actions, search offset and grep cursor contracts. @@ -84,89 +85,92 @@ not needed for choosing these text layouts; no optimization is part of scope. ## Selected output design -### Header +### Header — revised proposal after user feedback -Use the same ASCII ` | ` separator for outcome, tool-specific counts and short -qualifiers. Keep the headline free of request parameter names and readiness -fractions. Preserve pluralization and each tool's definition of its counts. +The earlier pipe-separated design is superseded. It counted one documentation +hit twice (`1 result | 1 docs page`) and used `partial` without saying what was +missing. The user contested this on 2026-10-06; do not implement that shape. + +Lead with one plain sentence describing this returned page. Count each search +result once, either as a known kind or in a mixed-kind breakdown; do not add a +redundant total. Documentation results are returned hits, not a newly invented +count of unique URLs (multiple sections can come from one page). ```text -1 result | 1 docs page | partial | more available -1 match in 1 line across 1 page | more available -3 results | 2 repo code hits, 1 docs page -4 matches in 3 lines across 2 files | coverage incomplete +Found 1 documentation result. +Found 2 code results and 1 documentation result. +Found 4 matches on 3 lines in 2 files. ``` -Search `partial` follows backend partialResults for active and completed pages. -An active, non-partial snapshot with hits uses `interim` instead. `more available` -follows hasMore / nextCursor and never implies exhaustive coverage. -For grep, classify gaps from the existing facts before choosing a headline: - -- Actual coverage gaps: a scope has readiness other than CURRENT, excluding - UNSPECIFIED paired with RESUMABLE_LIMIT (documented unvisited pagination); - traversal other than COMPLETE/RESUMABLE_LIMIT, an error or recorded scan omissions/issues, - or overall traversal is NON_RESUMABLE_PARTIAL, FAILED or CURSOR_EXPIRED. -- Retryable omissions only: at least one unavailableTarget, every omission is - retryable, and no actual coverage gap above. This includes preparing work and - existing retryable non-preparing reasons. Use the pagination exception in the - formatter-local omissions-only classification. -- Non-retryable omissions: any unavailableTarget with retryable=false. - -Hit-bearing pages use `partial` for retryable omissions only, and -`coverage incomplete` for any actual coverage gap or non-retryable omission -(the stronger qualification wins, so never print both). Append `more available` -independently for a supplied nextCursor. Both CURRENT+RESUMABLE_LIMIT and -UNSPECIFIED+RESUMABLE_LIMIT with no error/scan issues are ordinary pagination -and add neither partial nor incomplete-coverage copy. Apply this exemption -consistently to headline, omissions-only and zero-page decisions. Preserve the -unvisited source row and its existing no-results-on-this-page qualifier; do not -pretend that unvisited content was searched. -Keep the existing exhaustive predicate for claiming a full no-match search. -Do not change service validation or source-evidence projection. - -Search prints lifecycle separately immediately below its headline when active, -terminal or unknown; completed search needs no lifecycle line: +Search labels should distinguish repository documentation, hosted documentation +and symbols when supplied by existing hit kinds; unknown kinds retain a plain +result count rather than acquiring an invented classification. Grep's matches, +lines and files/pages are different quantities, so retain their useful relation +in the sentence. Both start with Found and use normal pluralization. -```text -1 result | 1 docs page | partial | more available -Search: indexing -``` +Do not append `partial`, `interim`, readiness fractions or pagination parameters. +More results belongs solely in its footer. Preparation and limitations are +explained in sentences or their existing attributed sections, not compact flags. +For the captured documentation-ready/code-pending case, the proposed anatomy is: -This preserves INDEXING/SEARCHING/PENDING/deferred/timeout/failed/unknown distinctions -without presenting target counts as result readiness. Grep has no persistent -search session and does not invent this line. Existing grep cursor-expiry and -scope coverage explanations remain before matches. +```text +Found 1 documentation result. -Zero/no-snapshot outcomes keep their precise meanings: +Sources: + - site:expressjs.com (hosted documentation) -- Completed search with no hits: `No results`; when hasMore is true: - `No results on this page | more available`. Add `partial` before the pagination - clause when the actual snapshot has partialResults=true. -- Active search with an empty snapshot: `No results yet`; absent snapshot: - `No result snapshot yet`, each followed by its Search lifecycle line. Empty - snapshots append `| partial` when true; absent snapshots cannot claim partial. -- Terminal/unknown search keeps `No results` versus `No result snapshot`, followed - by its explicit lifecycle line and existing recovery disposition. Retained - empty snapshots keep the same partial qualifier when supplied. +Preparing: + - github:expressjs/express@1bb798d9 (indexing, estimated total: 25-61s) + Requested: npm:express@2.3.10 +``` -Grep's zero-hit headlines use these exact cases (pagination is independent): +If a brief lifecycle explanation is needed, use a sentence with its actual +meaning, for example `Repository code is still indexing; these documentation +results are usable now.` Only name repository code when supplied source/work +facts establish it; a preparing refresh does not prove no code was searched. +Avoid repeating an equivalent existing Preparing/scope explanation. Preserve +existing use-now and conditional-wait wording in the footer. + +Search backend partialResults remains meaningful, including empty/completed +snapshots. If attributed source/preparation/coverage notes already explain the +missing scope, do not repeat an abstract warning. If partialResults=true has no +such explanation, say `These results do not cover the full request.` without +guessing an indexing cause. Retain this fact in the private availability model; +public JSON stays unchanged. Active work not explained by Preparing can say +`Search is still running.` Known terminal states retain explicit ended/failed +reason sentences and unknown states remain unknown; do not imply completion. + +Grep classification keeps the reviewed distinction, but it now drives prose and +empty outcomes rather than abstract headline qualifiers: + +- Actual gaps: readiness other than CURRENT except the documented unvisited + UNSPECIFIED+RESUMABLE_LIMIT case; non-pagination traversal, errors, skips and + scan issues, or overall NON_RESUMABLE_PARTIAL/FAILED/CURSOR_EXPIRED. +- Only retryable omissions: no actual gap, at least one unavailableTarget and + all omissions retryable. Preparing/Omitted rows explain the temporary omission. +- Non-retryable omissions or actual gaps: existing attributed coverage/reason + notes explain the limit. When no existing note conveys the overall limitation, + use `Some requested content could not be searched.` without inventing a cause. +- CURRENT+RESUMABLE_LIMIT and unvisited UNSPECIFIED+RESUMABLE_LIMIT without + independent errors/skips are ordinary pagination, not failures. Keep unvisited + source qualifiers and the exact cursor; no unnecessary retry. + +Preserve the strict exhaustive predicate. The revised zero-page examples are: | Returned page | Outcome | | --- | --- | -| Exhaustive, no omissions, no cursor | `No matches.` | -| Only retryable omissions, no cursor | `No matches yet.` | -| No other coverage gap or omission, valid cursor | `No matches on this page | more available` | -| Only retryable omissions, valid cursor | `No matches yet on this page | more available` | -| Scope/scan/traversal failure or any non-retryable omission, no cursor | `Zero returned matches; coverage is incomplete.` | -| Same incomplete case, valid cursor | `Zero returned matches | coverage incomplete | more available` | - -Retryable omissions include preparing repository/docs work as well as existing -retryable non-preparing reasons. Preparing is not called a failure. Sources, -Preparing and Omitted rows retain the exact reason and target attribution. -The captured hit-bearing Express docs/pending-code example therefore becomes -`1 match in 1 line across 1 page | partial | more available`; -its zero-hit equivalent follows the fourth row, without losing either "yet" or -the available continuation. No supplied cursor is silently hidden. +| Exhaustive search/grep | `No results found.` / `No matches found.` | +| Active search or only retryable grep omissions | `No results available yet.` / `No matches available yet.` | +| Empty continuation page | `No results on this page.` / `No matches on this page.` | +| Empty continuation plus retryable grep omissions | `No matches available yet on this page.` | +| Actual missing/failed scope | Plain no-results/no-matches outcome plus the attributed limitation explanation | +| No search snapshot | `No results available yet.` while active, or explicit ended-search explanation otherwise | + +Pagination remains independently available under More results. Partial empty +snapshots must not imply an exhaustive no-result search; use the scope explanation +or the full-request warning above. These new copy choices are proposed and need +review once the user settles the headline shape; prior reviews covered the +semantics, not this revised wording. ### Body and source sections @@ -276,8 +280,10 @@ A Commander-level helper would duplicate MCP behavior; a core helper would put presentation in transport. Neither is appropriate. Use one small pure `packages/mcp/src/shared/search-grep-output-text.ts` helper for -` | ` headline joining/wrapping/emphasis and the fixed Read/More results/Follow-up -section skeleton. Its inputs are already-rendered headline clauses and optional +the fixed Read/More results/Follow-up section skeleton only. Headline sentences +reuse existing terminal prose wrapping and emphasis in each tool formatter; +there is no reason for a separate shared counter/joining abstraction. +Its inputs are optional read/more/follow-up lines; it knows no backend state, target identity or command arguments. Treat action lines as verbatim strings; only formatter-authored prose is wrapped before supplying it. No configurable section registry, formatter DSL, @@ -307,21 +313,22 @@ contracts; this change concerns successful/retained result text, not auth errors ## Phase 1 — consistent and truthful result edges -- Status: **READY**. +- Status: **DESIGN REVISION**. - Expected outcome: the examples above hold for CLI/MCP search/status and grep; usable results, pending scopes and available actions are immediately clear. - Assumptions: existing nextOffset/cursor/read/lifecycle facts suffice (verified above); fixed three-section helper needs no new service data; long cursors remain unavoidable within the existing contract. -- Unknowns/product decisions: **none**. Implementation evidence may expose a - contradiction; report it before widening the scope or changing the design. +- Unknowns/product decisions: settle revised headline wording and its concrete + examples before implementation. Then review the revised design; no production + changes while this is open. - Dependencies: reviewed plan, merged source rows and current main baseline. Ordered implementation: 1. Add behavioral fixtures for headers and independently optional footer actions in existing formatter tests. Add the small shared helper and integrate search - outcome/lifecycle lines and grep separator/coverage clauses. + plain outcome sentences and evidence-based limitation explanations. 2. Separate search read/pagination/follow-up rendering while preserving its semantic action projection; integrate grep's existing actions through the same skeleton. Preserve exact locators/cursors/wait units and all target recovery/body output. @@ -338,13 +345,15 @@ Ordered implementation: Acceptance cases: - Ready search code/docs/mixed hits and ready grep multiple occurrences on one - line: counts are correct, JSON unchanged, same separators/section labels. + line: counts are correct and not repeated, JSON unchanged, plain sentences and + the same footer section labels; no pipe-separated counters or unexplained flags. - Ready docs plus pending code, including multiple targets and duplicate aliases: Sources/Preparing stay truthful; no ambiguous readiness fraction; search and - grep both say partial for usable docs while code prepares; grep normal + grep explain pending work through attributed Preparing facts rather than an + unexplained partial label; grep normal pagination alone never claims incomplete coverage. -- Active non-partial interim and completed partial search: retain interim/partial - truth independently of completed state. PENDING/INDEXING/SEARCHING and +- Active interim and completed partial search: explain continuing work and + incomplete request coverage in plain language independently of completed state. PENDING/INDEXING/SEARCHING and DEFERRED/TIMEOUT/FAILED/unknown remain visible and receive only their valid actions. - Empty complete, empty active, absent snapshot, zero-hit continuation pages, withheld scopes, actual coverage issues and expired grep cursor: precise @@ -419,7 +428,10 @@ this user-selected two-tool follow-up is an explicit narrow cross-tool increment One phase means no intermediate merge/reorientation boundary. If verified evidence requires another phase or a broader design, stop and replan with the user. -Internal technical review is **clean** after correcting empty partial-search +Historical review of the superseded pipe-separated proposal follows. It does +not establish readiness of the current headline revision. + +Internal technical review was **clean** after correcting empty partial-search provenance, empty resumable grep wording and the known terminal status list. The private availability correction is the smallest boundary change needed; public JSON and service contracts stay unchanged. External Claude round 1 accepted direction and found five plan corrections: @@ -441,3 +453,11 @@ scopes beside the matched source|retains an unvisited selected site'` — 2 pass All findings are corrected; no direction, scope or product question remains. There was no finding-free external round within the cap; implementation will receive its own fresh review loop and actual output verification. No production change or planning-only PR created. + +## User-directed headline revision (2026-10-06) + +The previous READY state and pipe-separated examples are superseded by the +feedback above. The underlying reviewed continuation/coverage semantics and +footer design remain available; headline copy and the smaller footer-only helper +are proposed, not reviewed-ready. Do not treat historical review closure as +approval of the revised words. No production code changed. From 883d1045a27bfb583579fcf3343d4e9f89ecf7ce Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Wed, 7 Oct 2026 09:42:09 +0300 Subject: [PATCH 4/8] docs: constrain repeated result output prose Record the user requirement to avoid per-call token overhead from restating Sources and Preparing. Keep one outcome sentence by default and preserve conditional wait guidance only where it changes the next action. --- docs/plans/search-grep-headers-footers.md | 38 +++++++++++++++++------ 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/docs/plans/search-grep-headers-footers.md b/docs/plans/search-grep-headers-footers.md index 2b6953cd..18391ff7 100644 --- a/docs/plans/search-grep-headers-footers.md +++ b/docs/plans/search-grep-headers-footers.md @@ -124,12 +124,20 @@ Preparing: Requested: npm:express@2.3.10 ``` -If a brief lifecycle explanation is needed, use a sentence with its actual -meaning, for example `Repository code is still indexing; these documentation -results are usable now.` Only name repository code when supplied source/work -facts establish it; a preparing refresh does not prove no code was searched. -Avoid repeating an equivalent existing Preparing/scope explanation. Preserve -existing use-now and conditional-wait wording in the footer. +#### Per-call concision constraint (user feedback, 2026-10-07) + +Default to exactly one outcome sentence. Sources and Preparing already carry +served scope and pending work; do not add a second sentence paraphrasing them. +In the captured ready-documentation/pending-code case above, there is no extra +`Repository code is still indexing` or `documentation results are usable now` +paragraph. Repetition on every call accumulates unnecessary agent context. + +Add at most one short explanation only when a meaningful limitation or lifecycle +fact is otherwise missing. The notice must add a fact, not restate a label or +teach the output format. Name a cause only when supplied facts establish it; +a preparing refresh does not prove no code was searched. Preserve the deliberate +use-now/conditional-wait guidance when actually offering a wait alongside usable +hits; healthy results without wait advice need no use-now explanation. Search backend partialResults remains meaningful, including empty/completed snapshots. If attributed source/preparation/coverage notes already explain the @@ -197,8 +205,9 @@ No section is printed without a real action or meaningful advisory. Header and labels use identical wording on CLI and MCP; action syntax remains native. Search selects the first actual returned read action as an example even on a -healthy completed page. Lead with `Use these results now; example read:` rather than -implying it reads every result. Grep retains file/page templates, removes +healthy completed page. On a healthy page with no wait advice, show the read +command without instructional prose. When a wait is also offered, retain the +short `Use these results now; example read:` lead to prevent wait-first behavior. Grep retains file/page templates, removes only their leading `#` and prefixes CLI templates with `githits` so they are consistent command recipes; no template appears for an empty page. Example: @@ -350,8 +359,8 @@ Acceptance cases: - Ready docs plus pending code, including multiple targets and duplicate aliases: Sources/Preparing stay truthful; no ambiguous readiness fraction; search and grep explain pending work through attributed Preparing facts rather than an - unexplained partial label; grep normal - pagination alone never claims incomplete coverage. + unexplained partial label; no extra sentence restates Preparing or the usable + Sources. Grep normal pagination alone never claims incomplete coverage. - Active interim and completed partial search: explain continuing work and incomplete request coverage in plain language independently of completed state. PENDING/INDEXING/SEARCHING and DEFERRED/TIMEOUT/FAILED/unknown remain visible and receive only their valid actions. @@ -373,6 +382,10 @@ Acceptance cases: never invent a read target. - Prior HEAD, current HEAD and ended evidence: read-now before optional wait, exact specific-ref advice, no pollable ended search. +- Repeated-call concision: ordinary ready and ready-docs/pending-code pages + contain one outcome sentence, no redundant status explanation, and action-only + healthy read guidance. Exceptional notes add otherwise missing facts; conditional + waits with usable results retain the behavioral use-now safeguard. - 40/80/120-column output, ANSI stripped versus plain output and backend Unicode: prose wraps, fixed actions/content do not; all footer sections omit cleanly. @@ -461,3 +474,8 @@ feedback above. The underlying reviewed continuation/coverage semantics and footer design remain available; headline copy and the smaller footer-only helper are proposed, not reviewed-ready. Do not treat historical review closure as approval of the revised words. No production code changed. + +The user additionally requires per-call token discipline on 2026-10-07: default +header-only outcome, no paraphrase of Sources/Preparing, and exceptional prose +only for otherwise undisclosed facts. This refines the proposed copy and +acceptance; it is not an output-token reduction claim before implementation. From ed6a0893dc514fff922ecfe23e44902da188428c Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Wed, 7 Oct 2026 10:07:01 +0300 Subject: [PATCH 5/8] fix: unify concise search and grep output Use plain outcome counts and shared Read, More results and Follow-up footers. Retain exact native actions, source preparation and coverage limitations while keeping JSON and request behavior unchanged. --- changes/concise-search-grep-output.changed.md | 6 + docs/implementation/mcp-cli-parity.md | 46 ++- .../search-snapshot-presentation.md | 85 ++++- docs/implementation/tools.md | 69 ++-- docs/implementation/unified-grep.md | 30 +- docs/plans/search-grep-headers-footers.md | 67 +++- packages/mcp/src/shared/grep-response.test.ts | 121 ++++++- .../src/shared/grep-text-rendering.test.ts | 32 +- packages/mcp/src/shared/grep-text.test.ts | 42 +-- packages/mcp/src/shared/grep-text.ts | 124 ++++--- .../mcp/src/shared/indexing-estimates.test.ts | 6 +- .../mcp/src/shared/search-grep-output-text.ts | 38 ++ .../unified-search-presentation.test.ts | 4 + .../src/shared/unified-search-presentation.ts | 16 +- .../unified-search-semantic-text.test.ts | 6 +- .../unified-search-snapshot-text.test.ts | 48 +-- .../shared/unified-search-status-text.test.ts | 58 ++- .../src/shared/unified-search-text.test.ts | 264 ++++++++------ .../mcp/src/shared/unified-search-text.ts | 333 +++++++----------- packages/mcp/src/smoke-test.test.ts | 107 +++--- packages/mcp/src/smoke-test.ts | 73 ++-- packages/mcp/src/tools/grep.test.ts | 6 +- packages/mcp/src/tools/search-status.test.ts | 46 +-- packages/mcp/src/tools/search.test.ts | 2 +- scripts/cli-smoke.ts | 61 ++-- scripts/smoke-scripts.test.ts | 116 +++--- src/commands/search.test.ts | 120 +++---- src/tools/search-parity.test.ts | 34 +- 28 files changed, 1165 insertions(+), 795 deletions(-) create mode 100644 changes/concise-search-grep-output.changed.md create mode 100644 packages/mcp/src/shared/search-grep-output-text.ts diff --git a/changes/concise-search-grep-output.changed.md b/changes/concise-search-grep-output.changed.md new file mode 100644 index 00000000..1dad40c1 --- /dev/null +++ b/changes/concise-search-grep-output.changed.md @@ -0,0 +1,6 @@ +--- +"githits": patch +"@githits/mcp": patch +--- + +- **Concise search and grep output** - Use plain outcome counts and shared Read, More results and Follow-up footers, retaining exact native actions, source/preparation provenance and meaningful coverage warnings without repeated header flags. JSON and request behavior are unchanged. diff --git a/docs/implementation/mcp-cli-parity.md b/docs/implementation/mcp-cli-parity.md index 9b3d3603..a1734096 100644 --- a/docs/implementation/mcp-cli-parity.md +++ b/docs/implementation/mcp-cli-parity.md @@ -369,25 +369,17 @@ readiness, trust limits, and action selection; the text renderer owns wording, wrapping, hit anatomy, and ordering. Callers provide ANSI enablement, surface-native action syntax, and an optional output width. CLI supplies its current terminal width; MCP uses the formatter's 80-column default. The order is -an outcome headline carrying count/breakdown, lifecycle, readiness, and -pagination when applicable; shared `Sources:` bullet rows for searched evidence, +one plain outcome sentence counting each returned kind once, without lifecycle, +readiness fractions or pagination; shared `Sources:` rows for searched evidence, then `Preparing:` rows for actual work before ranked hits. Known repository rows -use an 8-character display SHA, optional independent date and historical ref; -current healthy evidence uses the same shape. Documentation rows retain exact -site scope/corpus and package attribution. Unknown identities retain supplied -labels. Zero-hit searched sources disclose their scope; unsearched/withheld -sources retain target-local readiness/recovery instead. Different full SHAs, -corpora, readiness or coverage remain distinct. Target-local limitations and -global warnings remain visible, followed by at most one final `Next:` action. - -`PENDING`, `INDEXING`, and `SEARCHING` remain distinct. Active empty output uses -`No results yet | indexing | 0/1 ready`; an active response without a snapshot -uses `No result snapshot yet | indexing | 0/1 ready`, with corresponding -lower-case lifecycle labels for other active states. Active result counts use -`partial` only when `partialResults` is true; otherwise they say `results` -beside the lifecycle. Terminal -or unknown progress retains lifecycle/readiness in the headline, while completed -output omits them. Target rows keep deterministic `commit`/`using`, `searched`, +use an 8-character display SHA, optional independent date and historical ref. +Target/source limitations remain attributed. Backend partialResults gets a short +full-request warning only when those rows do not explain the missing scope. +Terminal and unknown states retain explicit prose; active work gets a notice +only when otherwise unexplained. Empty active output says `No results available +yet.`; empty continuation pages say `No results on this page.` + +Target rows keep deterministic `commit`/`using`, `searched`, `indexing`, terminal/unavailable, `available`, `indexed`, and constraint segments; exact terminal reasons remain lane-readable, and a target gets at most one inline `Fix:` or replayable `Try:` line. Site suggestions and indexed alternatives @@ -401,14 +393,16 @@ codes, indexing references, and opaque evidence text stay out of default text. Reissuing the same search is valid and waits on the same underlying work; text does not emit negative repeat or poll policy directives. -MCP renders `Next: search_status search_ref=... wait_timeout_ms=...`; CLI renders -`Next: githits search-status ... --wait ...`. An active continuation reference -appears exactly once, in that surface-native final `Next:` action; stopped terminal -references are not rendered. Raw diagnostic fields are never rendered. -Search results omit per-hit read commands from both text surfaces. ANSI-stripped -CLI output shares the same hierarchy and wording as no-color MCP text; line -breaks can differ because CLI uses the terminal width while MCP uses the -80-column default. +Search and grep share optional `Read:`, `More results:` and `Follow-up:` footers +in that order. Search offers one exact read example, including healthy completed +results; grep offers file/page templates after matches. MCP actions use native +`read`, `search_status` and argument syntax; CLI actions use `githits` commands. +A status reference appears once under Follow-up. With usable hits, reading is +first and waiting conditional; stopped references never poll. Pagination repeats +the original search with its exact offset or grep with its opaque cursor, +preserving original controls. Search status cannot paginate. ANSI changes only +emphasis; widths can differ. See [snapshot presentation](search-snapshot-presentation.md#concise-search-and-grep-headers-and-footers) +for zero-page, limitation and continuation rules. JSON remains unchanged. Documentation JSON retains `docsReadTarget`, compatible `pageId`, and provenance `sourceUrl`. Search/status JSON follow-ups consume the backend `ReadTarget` diff --git a/docs/implementation/search-snapshot-presentation.md b/docs/implementation/search-snapshot-presentation.md index e52a699d..606345ea 100644 --- a/docs/implementation/search-snapshot-presentation.md +++ b/docs/implementation/search-snapshot-presentation.md @@ -17,11 +17,16 @@ stored and could not obtain later evidence. With returned hits, an active search now leads with: ```text -Next: use these hits now; read for details: -read target="github:anomalyco/opencode@bbd72fb8" path="..." start_line=480 end_line=490 -For a specific ref, search github:anomalyco/opencode@. -If you need current HEAD, wait (hits and order may change): -search_status search_ref="..." wait_timeout_ms=120000 +Found 1 code result. + +Read: + Use these results now; example read: + read target="github:anomalyco/opencode@bbd72fb8" path="..." start_line=480 end_line=490 + For a specific ref, search github:anomalyco/opencode@. + +Follow-up: + If you need current HEAD, wait (hits and order may change): + search_status search_ref="..." wait_timeout_ms=120000 ``` The specific-ref advice and HEAD-specific conditional appear only for proved @@ -502,3 +507,73 @@ guideline against default-true agent booleans, preserving the existing flag. Compact query echo omits true as a default; false remains explicit. The parameter/status description changes are in this same user-directed increment; qualitative eval authentication limits above remain unchanged. + + +## Concise search and grep headers and footers + +The shared `search-grep-output-text.ts` owns only the fixed footer order and +indentation: optional **Read**, **More results**, then **Follow-up**. Each tool +renderer owns its outcome, limitations, lifecycle and exact native operands. +There is no command reconstruction or new service/state layer. Initial search +and search status continue to share their semantic projection and renderer. + +Search counts each returned result once by kind, for example `Found 2 code +results and 1 documentation result.` Grep counts page occurrences, distinct +physical lines and exact file/page identities: `Found 4 matches on 3 lines in +2 files.` Headlines omit pipe separators, readiness fractions, pagination and +abstract partial/interim labels. Sources/Preparing and attributed coverage notes +explain available evidence and missing scope. Only otherwise unexplained facts +get a short notice: `These results do not cover the full request.` for backend +partialResults, or explicit running/terminal/unknown lifecycle prose. The private +availability projection retains partialResults, including completed empty +snapshots; public JSON is unchanged. Dates do not drive these decisions. + +Healthy searches offer one exact read example without use-now prose. When a wait +is offered beside usable hits, the read comes first with short use-now advice; +the wait remains conditional. Readless usable results put `Use these results now.` +first in Follow-up. Grep templates follow all matches; CLI templates include +`githits`. Exact operands are never split by prose wrapping, and ANSI changes +only emphasis. Empty outputs omit read advice. + +Pagination always lives in More results, independently of lifecycle. Search/status +instruct repeating the original search with the exact next offset, preserving +its target/query/filter controls; status itself cannot paginate. A missing offset +gets a truthful availability hint with no invented value. Active search pages +warn that results can change. Grep preserves the opaque cursor for the original +ordered targets and controls. Follow-up retains seconds for CLI search-status +and milliseconds for MCP status and grep retries. Ended references never poll. +Counted alternatives say `(+N more)`; unknown `+more` remains unknown. + +For grep, CURRENT+RESUMABLE_LIMIT and UNSPECIFIED+RESUMABLE_LIMIT are ordinary +pagination absent independent errors/skips. The strict exhaustive predicate +still controls source-level `no results` claims. Retryable-only omitted targets +use `No matches available yet.` (adding `on this page` beside a cursor), even +when omission accounting yields NON_RESUMABLE_PARTIAL overall. Preparing/Omitted +already explains that limit. Independent failed traversal, stale readiness, +errors/skips/issues and expired cursors retain attributed warnings. Unspecified +readiness outside unvisited pagination is explicitly unknown. Search/grep empty +continuation pages say `No results on this page.` / `No matches on this page.` + + +Follow-up verification (2026-10-07): full `bun test` passes 5,624 tests across +235 files, zero failures and 22,404 assertions. Typecheck, both builds, source +and built CLI/MCP smoke checks, and packed public-package validation pass. Smoke +business cohorts skipped AUTH_REQUIRED; authenticated dev calls separately +prove output. Pending CLI search/grep used registry-confirmed Express 2.3.12; +MCP grep used 2.4.0 and fresh pending MCP search/status used 2.4.1. Both pending +search surfaces returned one hosted-doc result with actual repository Preparing, +indexed alternatives, native read, offset1 and conditional status. Immediate +MCP status returned byte-identical text. Once indexed, healthy CLI/MCP search +returned code with Sources, Read and More results, without a wait. Initial MCP +search's transient Keychain error was retried successfully; no auth code changed. + +The same fixed renderer fixtures measure ready search 89 -> 163 bytes (+74), +preparing search 459 -> 553 (+94) and mixed paged grep 6992 -> 6985 (-7). Search +adds actionable read/pagination footers the baseline omitted. These are output +bytes, not tokens, latency or proof of agent quality. Targeted Claude agent:e2e +search-investigation and grep-mixed-docs remained blocked by `Not logged in`: +zero tool calls, no final/isolation artifacts, unknown usage. No comprehension +claim follows. Internal review accepted and closed an overall grep traversal +warning hidden by a sibling cursor and two stale documentation paragraphs. +The eight-file closure passes 244 tests, zero failures and 913 assertions; no +new infrastructure or major deferred item. External review result follows. diff --git a/docs/implementation/tools.md b/docs/implementation/tools.md index 9c24e6df..48286d3e 100644 --- a/docs/implementation/tools.md +++ b/docs/implementation/tools.md @@ -264,7 +264,7 @@ Treat failures as live backend or contract findings, not deterministic unit-test **Unified `search` query syntax.** The `search.query` field is the backend discovery query syntax, not a raw pass-through to a per-source search engine. It supports implicit `AND`, uppercase `OR`, parentheses, unary `-`, quoted phrases, semantic qualifiers (`kind:`, `category:`, `path:`, `lang:`, `name:`, `intent:`), and routing qualifiers (`registry:`, `package:`, `version:`, `repo:`). MCP callers put these constraints directly in `query`; the backend owns parsing, current enum validation, recovery warnings, and per-source compilation. Per-source support, ignored features, and incompatibilities are reported in `sourceStatus`. CLI users retain `--kind`, `--category`, `--path-prefix`, `--intent`, `--name`, and `--lang`; the shared request builder adapts those human-facing flags to the same backend operation. -**Partial-result truth.** Search defaults to partial results so ready sources can contribute while other target/source pairs prepare. Explicit `allow_partial_results: false` requires atomic evidence across runnable pairs. Every result-bearing initial `search` payload and stored `search_status.result` carries the backend's exact `partialResults: boolean`, including `false` for an atomic serveable interim snapshot and `true` for a subset of requested evidence. A progress-only response with no result snapshot omits the field. This additive field is retained unchanged in CLI `--json` and MCP `format: "json"`; text-v1 labels active results as `partial` only when it is true; otherwise the adjacent lifecycle identifies background work. +**Partial-result truth.** Search defaults to partial results so ready sources can contribute while other target/source pairs prepare. Explicit `allow_partial_results: false` requires atomic evidence across runnable pairs. Every result-bearing initial `search` payload and stored `search_status.result` carries the backend's exact `partialResults: boolean`, including `false` for an atomic serveable interim snapshot and `true` for a subset of requested evidence. A progress-only response with no result snapshot omits the field. This additive field is retained unchanged in CLI `--json` and MCP `format: "json"`; text-v1 explains missing scope through attributed source/preparation notes or a short full-request warning, including completed empty snapshots; it does not label headlines `partial`. **Repository search evidence locators.** Repository code and symbol hits keep the legacy target-relative `locator.filePath` and evidence `startLine` / `endLine` while also exposing the repository-root `repositoryFilePath`, exact served `commitSha`, explicit `evidenceRange`, original `indexedRange`, and optional `symbolContext`. Evidence includes `matchLine`, backend `rangeKind`, and `matchSpansTruncated`; symbol context keeps backend identity/kind plus the fixed lowercase relation `encloses_match` or `associated_with_indexed_chunk`. A proven enclosing relation always has one complete `definitionRange` containing both target-relative and repository-root paths. Associated or identity-only context may omit that range. Malformed partial definition locators invalidate the search response instead of being repaired or dropped. @@ -395,14 +395,13 @@ notes and reason enums are not copied into text. Query-wide warnings remain one global `Warnings:` block after target rows and before hits; target-owned constraints stay in their target row and unowned source constraints remain global. -There is at most one final `Next:` line. Active continuation uses the supplied -`searchRef` exactly once in the executable `search_status` action; there is no -separate session row. MCP renders -`Next: search_status search_ref="..." wait_timeout_ms=30000` when no range is -available; supported indexing ranges select a bounded wait as described below. A target-local -`Fix:`/`Try:` never suppresses an active poll or completed evidence-status action, -but suppresses generic rerun/query-rewrite guidance. Terminal and unknown sessions -do not poll their stopped reference. Reissuing the same search remains valid. +Optional footers use `Read:`, `More results:` and `Follow-up:` in that order. +Active continuation uses the supplied `searchRef` once under Follow-up, with +native `search_status search_ref="..." wait_timeout_ms=30000` syntax when no +range is available. Supported indexing ranges select a bounded wait. Usable +hits get a read first and a conditional wait. Terminal/unknown sessions require +a fresh search, never polling their stored reference. Target-local recovery +suppresses generic rerun/query-rewrite advice, not pagination or active status. JSON remains the lossless stable boundary: `sourceStatus`, warnings, target resolution, evidence notices, and hit metadata are retained there even when text @@ -763,21 +762,18 @@ cost savings. Captures and reproduction scripts are under ignored groups and trust facts; one shared text renderer owns wording, wrapping, hit anatomy, and ordering. The order is: -1. one outcome headline with count/breakdown, lifecycle, readiness, and - pagination when applicable; -2. compact served-identity bullets under `Sources:`, then actual-work bullets - under `Preparing:` when present, retaining concrete provenance and aliases; -3. target-local state and recovery, then query-wide warnings; -4. the separate numbered ranked hit list; and -5. at most one session/query-wide `Next:` action. - -Active empty headlines are `No results yet | indexing | 0/1 ready` and -`No result snapshot yet | indexing | 0/1 ready` (with `preparing` or `searching` -for the other active states). Active results say `partial` only when -`partialResults` is true; otherwise they say `results` beside the lifecycle. Terminal or unknown -progress retains its lower-case lifecycle and readiness; completed output omits -those fields. Progress-only responses show only derivable target identity and -lane-free freshness; they never invent source or contributor facts. +1. one plain outcome sentence, for example `Found 1 documentation result.`; +2. served identities under `Sources:`, then actual work under `Preparing:`; +3. target-local limitations/recovery, then query-wide warnings; +4. numbered ranked hits; and +5. optional `Read:`, `More results:` and `Follow-up:` footers in that order. + +Headlines contain no pipe-separated lifecycle, readiness fractions, pagination +or partial/interim labels. Sources/Preparing explain missing scope; unexplained +backend partialResults adds `These results do not cover the full request.` +Terminal and unknown states retain explicit prose. Active empty output says +`No results available yet.`; continuation pages say `No results on this page.` +Progress-only responses never invent source or contributor facts. Detailed target rows keep one identity and deterministic segment order: remaining `using`, `searched`, `indexing`, terminal/unavailable, `available`, `indexed`, @@ -791,14 +787,12 @@ provisional, and coverage facts qualify the target/source rather than creating a second list. Query-wide warnings remain one `Warnings:` block after target rows and before hits; target-owned constraints stay in their row. -There is no separate session row. An active or evidence-status continuation uses -the supplied `searchRef` exactly once in the executable `Next:` action: -`Next: search_status search_ref="..." wait_timeout_ms=30000` for MCP or -`Next: githits search-status ... --wait 30` for CLI when no range is available. -Active indexing estimates can adjust that wait up to 120 seconds; completed -evidence-status retrieval keeps the default. Target-local recovery never -suppresses an active poll or completed evidence-status action, but suppresses a -generic rerun/query rewrite. Stopped terminal references are not polled. +There is no separate session row. Follow-up contains the active reference once: +`search_status search_ref="..." wait_timeout_ms=30000` for MCP or +`githits search-status ... --wait 30` for CLI when no range is available. +Indexing estimates can adjust this wait up to 120 seconds. Usable hits get +short use-now advice and a read before conditional waiting. Healthy completed +hits get one exact read without that extra prose. Ended references never poll. `evidenceNotice` stays exact in JSON and is not rendered in default text. JSON is the lossless stable boundary for source statuses, target resolution, warnings, @@ -855,11 +849,12 @@ consistent two-space hit-body indent. If a title does not fit on the header line, the fixed locator prefix stays unwrapped with a trailing ` -`, and only the title continues on two-space-indented lines. -Result headlines combine count, type breakdown when completed, and pagination -when known, for example `10 results | 5 repo docs, 5 docs pages | next_offset=10`. -Breakdowns use `repo code hit(s)` and `repo symbol(s)` alongside `repo doc(s)` -and `docs page(s)`. When more results exist without a next offset, the final field is -`more available`. Pagination is not repeated as a bottom paragraph. +Result headlines count each returned kind once, for example `Found 5 repository +documentation results and 5 documentation results.` More results instructs +repeating the original search with its exact `offset` / `--offset`, preserving +query/targets/filters. Search status cannot paginate. Missing offsets receive a +truthful advisory without a fabricated value. Active pagination warns that +results can change. Counted indexed alternatives use `(+N more)`. **Follow-up — mutable hosted docs and crawled-doc section anchors.** Hosted/crawled `documentation_page` HTTP(S) targets address mutable current content, while repository documentation is separately snapshot-addressed. The backend descriptor selects target, optional path, selector and bounds; search preview/evidence coordinates remain independent. Both surfaces preserve descriptor bytes and render selectors separately instead of promoting provenance fragments or inventing heading bounds. MCP search actions narrow explicit path selections to 300 lines; pathless docs selections remain complete, and CLI text retains the full selection. Missing action metadata produces unavailable guidance. A sufficient search snippet needs no read. For a heading read, the backend resolves the heading and its full subtree through the next equal-or-higher heading and reports its absolute page range. Missing, duplicate, windowed/inexact, or unsupported sections return non-retryable `DOCUMENTATION_SECTION_UNRESOLVED` with a reason; they never become `NOT_FOUND` or a successful full-page read. Publisher-only IDs omitted during ingestion remain unavailable. The client never decodes or normalizes locator bytes and does not synthesize website slug rules. diff --git a/docs/implementation/unified-grep.md b/docs/implementation/unified-grep.md index f6138c63..7baf3638 100644 --- a/docs/implementation/unified-grep.md +++ b/docs/implementation/unified-grep.md @@ -16,7 +16,7 @@ githits grep -F -- '--foo' github:example/repository Pending repository and documentation preparation is described in plain language, with advisory total indexing duration when available and a fresh-request retry -action. Opaque progress IDs stay in JSON. An empty page says "No matches yet" +action. Opaque progress IDs stay in JSON. An empty page says "No matches available yet." only when retryable unavailable targets account for every coverage gap; independent failures, cursor expiry and skipped evidence retain their warnings. Partial hits and real continuation cursors remain usable. Retry with the same ordered targets, @@ -159,8 +159,9 @@ Sources: [2] https://expressjs.com/en/4x/api/ 51: ... -# Read files: read --lines $start-$end -- $target $path -# Read pages: read --lines $start-$end -- $url +Read: + Files: githits read --lines $start-$end -- $target $path + Pages: githits read --lines $start-$end -- $url ``` Read templates appear after all matches and before pagination/retry guidance, @@ -195,9 +196,11 @@ the source with `(no results on this page)` instead of a separate unvisited-scop message. JSON preserves the backend enum and full status. Stale/failed scopes, skips, issues, omitted issue counts, safety normalization -and unavailable targets stay visible on zero-hit pages. `No matches.` is -exhaustive only for complete traversal without coverage gaps. Other empty -pages report incomplete coverage. Cursors and terminal omissions can coexist; +and unavailable targets stay visible on zero-hit pages. `No matches found.` +with source `(no results)` is exhaustive only for complete traversal without +coverage gaps. Empty continuation pages say `No matches on this page.`; real gaps +retain attributed explanations. Overall non-pagination traversal still gets a +limitation notice even when a sibling cursor remains. Cursors and terminal omissions can coexist; both are shown. Continue with identical ordered operands and controls. `CURSOR_EXPIRED` is a successful result requiring explicit restart; retained sibling hits and omissions remain visible. @@ -280,3 +283,18 @@ printed once, with each requested target alias once and input-specific suggestio Grep does not fetch commit dates or resolved HEAD intent, so it does not claim either. Scan counts, skipped files, issues, cursors and backend read actions stay with their original scopes. + + +Search/grep share optional Read, More results and Follow-up footers in that +order. Grep leads with a page count such as `Found 4 matches on 3 lines in +2 files.` Multiple occurrences on one physical line count as separate matches +but one line; file/page groups use exact read identities. Sources/Preparing and +attributed failure notes explain limitations without an abstract partial label. +Healthy pagination does not claim a coverage failure, including unvisited +UNSPECIFIED+RESUMABLE_LIMIT targets. The strict exhaustive predicate still owns +source `no results` claims. Retryable-only omissions say `No matches available +yet.`; empty continuation pages say `No matches on this page.` (or `No matches +available yet on this page.` with retryable omissions). Independent gaps retain +their warning beside the exact cursor. Read templates are absent without hits; +cursor and retry instructions remain independent. Auth, requests and JSON are +unchanged. The shared helper owns only footer layout; grep owns these semantics. diff --git a/docs/plans/search-grep-headers-footers.md b/docs/plans/search-grep-headers-footers.md index 18391ff7..c1f2fed7 100644 --- a/docs/plans/search-grep-headers-footers.md +++ b/docs/plans/search-grep-headers-footers.md @@ -2,20 +2,19 @@ ## Status and destination -- Overall: **DESIGN REVISION — headline wording under discussion**. -- Phase 1: **DESIGN REVISION** — one implementation increment makes search/status and +- Overall: **IN PROGRESS — implementation authorized**. +- Phase 1: **IN PROGRESS** — one implementation increment makes search/status and grep headers and footers consistent on CLI and local/published MCP package text. -- Product decisions: settle the revised plain-language headline examples. On - 2026-10-06 the user rejected the pipe-separated counters and unexplained partial - label as ambiguous, including for agents. The earlier readiness is superseded. +- Product decisions: **none open**. The user approved the plain-language + direction, constrained repeated prose on 2026-10-07, and invoked $implement. + These instructions settle the previously proposed wording and concision rules. - Dependencies: merged PR #454 (`c71ffb5`), current main `bc295b3`, existing source/preparation facts, existing read actions, search offset and grep cursor contracts. Readers should immediately see what this page returned, what is preparing, and how to read, continue or obtain updated results. Search and grep retain their different evidence counts and continuation semantics within one visible anatomy. This is one -bounded PR with product changes, not a planning-only PR. No production edits -are authorized by this planning turn. +bounded PR with product changes, not a planning-only PR. Implementation is authorized; no merge or release. ## Verified baseline @@ -153,9 +152,9 @@ empty outcomes rather than abstract headline qualifiers: - Actual gaps: readiness other than CURRENT except the documented unvisited UNSPECIFIED+RESUMABLE_LIMIT case; non-pagination traversal, errors, skips and - scan issues, or overall NON_RESUMABLE_PARTIAL/FAILED/CURSOR_EXPIRED. + scan issues, or overall NON_RESUMABLE_PARTIAL/FAILED/CURSOR_EXPIRED not explained solely by omitted targets. - Only retryable omissions: no actual gap, at least one unavailableTarget and - all omissions retryable. Preparing/Omitted rows explain the temporary omission. + all omissions retryable. NON_RESUMABLE_PARTIAL is also emitted for omission-only pages; the existing Preparing/Omitted rows explain that temporary limit without another traversal warning. - Non-retryable omissions or actual gaps: existing attributed coverage/reason notes explain the limit. When no existing note conveys the overall limitation, use `Some requested content could not be searched.` without inventing a cause. @@ -176,8 +175,8 @@ Preserve the strict exhaustive predicate. The revised zero-page examples are: Pagination remains independently available under More results. Partial empty snapshots must not imply an exhaustive no-result search; use the scope explanation -or the full-request warning above. These new copy choices are proposed and need -review once the user settles the headline shape; prior reviews covered the +or the full-request warning above. These copy choices were settled by the user +and are now under implementation review; prior plan reviews covered the semantics, not this revised wording. ### Body and source sections @@ -322,15 +321,14 @@ contracts; this change concerns successful/retained result text, not auth errors ## Phase 1 — consistent and truthful result edges -- Status: **DESIGN REVISION**. +- Status: **IN PROGRESS**. - Expected outcome: the examples above hold for CLI/MCP search/status and grep; usable results, pending scopes and available actions are immediately clear. - Assumptions: existing nextOffset/cursor/read/lifecycle facts suffice (verified above); fixed three-section helper needs no new service data; long cursors remain unavoidable within the existing contract. -- Unknowns/product decisions: settle revised headline wording and its concrete - examples before implementation. Then review the revised design; no production - changes while this is open. +- Unknowns/product decisions: **none** after the user invoked $implement on the + revised design. Fresh implementation review covers the final approved wording. - Dependencies: reviewed plan, merged source rows and current main baseline. Ordered implementation: @@ -479,3 +477,42 @@ The user additionally requires per-call token discipline on 2026-10-07: default header-only outcome, no paraphrase of Sources/Preparing, and exceptional prose only for otherwise undisclosed facts. This refines the proposed copy and acceptance; it is not an output-token reduction claim before implementation. + + +## Implementation evidence (2026-10-07) + +Implemented inline under $implement on merged main bc295b3. Private availability +retains backend partialResults for completed/empty snapshots; no public JSON, +request/query selection, descriptors, auth or other commands changed. The shared +helper owns fixed footer layout only; tool renderers retain state and exact +operands. Grep gap checks reuse the same target predicate while the exhaustive +predicate remains stricter. No new infrastructure or major deferral. + +Focused closure: 297 pass, zero fail, 932 assertions across four renderer/smoke +contract files. Full verification passed 5,624 tests/zero failures/22,404 assertions across235 files; typecheck, both builds, source and built CLI/MCP smokes and packed public-package validation passed. Targeted +Claude agent:e2e search-investigation and grep-mixed-docs executions failed with +`Not logged in` before any tool calls. Both tool traces are empty, final/isolation +artifacts absent and usage unknown; no comprehension/quality claim follows. + +Same fixed fixtures before/after: ready search 89 -> 163 bytes (+74), preparing +search 459 -> 553 (+94), mixed paged grep 6992 -> 6985 (-7). Search adds useful +native read/pagination actions that the baseline omitted. This is output size, +not token count, latency or agent-quality evidence. + +Authenticated dev CLI express@2.3.12 (registry-confirmed) returned one docs hit +and one hosted-doc grep match with Sources, repository Preparing, then Read, +More results and Follow-up. Exact command operands, alias and indexed alternatives +remain. MCP grep express@2.4.0 showed the same hierarchy with native syntax. +Initial MCP search encountered a transient Keychain error; retry succeeded after +that version had indexed, proving healthy code Read+pagination without a wait. +Fresh pending MCP search express2.4.1 and immediate retained status succeeded with identical text, exact read selector/offset1/native status wait70000. Healthy CLI/MCP search returned code with Read+More and no wait. + + +Internal finding closure: direction sound. Overall NON_RESUMABLE_PARTIAL/FAILED +can retain a cursor (parser accepts this valid shape); the old no-cursor-only +warning hid the limitation in that case. Fixed overall warning selection while +retaining exact cursor and omission-only silence. Sibling scan covered all overall +traversal branches, target gap/exhaustive predicates, parser enum/cursor contracts, +zero/hit pages and related documentation. Added both statuses with zero/hit pages; +no service/state change or speculative mechanism. Two stale grep-doc outcome +paragraphs now match current prose. Internal revised-delta closure is clean; 244 tests/zero failures/913 assertions across eight affected files prove the fix. Closure typecheck, both builds and source/built CLI/MCP smoke checks passed. diff --git a/packages/mcp/src/shared/grep-response.test.ts b/packages/mcp/src/shared/grep-response.test.ts index 19dfe068..b88e4321 100644 --- a/packages/mcp/src/shared/grep-response.test.ts +++ b/packages/mcp/src/shared/grep-response.test.ts @@ -228,9 +228,9 @@ describe("unified grep result and text", () => { output.indexOf("[1] github:o/r@abc packages/x/lib/a.ts"), ).toBeLessThan(output.lastIndexOf("[2]")); expect(output).toContain( - "# Read files: read --lines $start-$end -- $target $path", + "Files: githits read --lines $start-$end -- $target $path", ); - expect(output).toContain("# Read pages: read --lines $start-$end -- $url"); + expect(output).toContain("Pages: githits read --lines $start-$end -- $url"); expect(output).toContain("[2] https://docs.test/p"); expect(output).not.toContain("inputs 1, 0"); expect(output).not.toContain("current content"); @@ -289,8 +289,8 @@ describe("unified grep result and text", () => { const output = formatGrepText( result({ hits: [], totalMatches: 0, targets: [scope] }), ); - expect(output).toContain("Zero returned matches"); - expect(output).not.toContain("No matches."); + expect(output).toContain("No matches found."); + expect(output).not.toContain("(no results)"); } const output = formatGrepText( result({ hits: [], totalMatches: 0, targets: [scopes[4]!] }), @@ -299,7 +299,7 @@ describe("unified grep result and text", () => { expect(output).toContain("2 additional file issue"); expect(output.replace(/\s+/g, " ")).toContain("safety normalization"); expect(formatGrepText(result({ hits: [], totalMatches: 0 }))).toContain( - "No matches.", + "No matches found.", ); }); it("shows cursor and terminal omissions together and requires explicit expiry restart", () => { @@ -326,7 +326,7 @@ describe("unified grep result and text", () => { ).toBe("crawl:1"); expect(output).toContain("Suggested site"); expect(output).toContain("--cursor 'opaque'"); - expect(output).toContain("repeat this grep"); + expect(output).toContain("Repeat the original grep"); expect( formatGrepText( result({ @@ -335,7 +335,7 @@ describe("unified grep result and text", () => { nextCursor: "opaque", }), ), - ).toContain("more available"); + ).toContain("More results:"); expect( formatGrepText( result({ traversal: "CURSOR_EXPIRED", unavailableTargets: [omission] }), @@ -392,3 +392,110 @@ describe("unified grep result and text", () => { expect(leadingDash).toContain("read --lines $start-$end -- $target $path"); }); }); + +describe("grep page and omission outcomes", () => { + it.each(["CURRENT", "UNSPECIFIED"] as const)( + "treats %s resumable targets as normal pagination", + (readiness) => { + for (const hits of [[], [hit]]) { + const output = formatGrepText( + result({ + hits, + totalMatches: hits.length, + targets: [{ ...target, readiness, traversal: "RESUMABLE_LIMIT" }], + traversal: "RESUMABLE_LIMIT", + nextCursor: "exact cursor", + }), + ); + expect(output.split("\n")[0]).toBe( + hits.length + ? "Found 1 match on 1 line in 1 file." + : "No matches on this page.", + ); + expect(output).toContain("More results:"); + expect(output).toContain("--cursor 'exact cursor'"); + expect(output).not.toMatch( + /coverage is incomplete|could not be searched|Traversal is incomplete|Follow-up:/, + ); + } + }, + ); + it.each([null, "cursor"])( + "explains retryable-only omissions without an exhaustive empty claim", + (nextCursor) => { + const output = formatGrepText( + result({ + hits: [], + totalMatches: 0, + nextCursor, + traversal: "NON_RESUMABLE_PARTIAL", + unavailableTargets: [ + { + inputIndex: 0, + target: "npm:pending", + reason: "repository_indexing", + retryable: true, + progressRef: null, + suggestedSiteTargets: null, + }, + ], + }), + ); + expect(output.split("\n")[0]).toBe( + nextCursor + ? "No matches available yet on this page." + : "No matches available yet.", + ); + expect(output).toContain("Preparing:"); + expect(output).toContain("Follow-up:"); + expect(output).not.toContain("Traversal is incomplete"); + }, + ); + it("retains attributed gaps even beside a resumable cursor", () => { + const output = formatGrepText( + result({ + hits: [], + totalMatches: 0, + targets: [ + { ...target, binaryFilesSkipped: 1, traversal: "RESUMABLE_LIMIT" }, + ], + traversal: "RESUMABLE_LIMIT", + nextCursor: "cursor", + }), + ); + expect(output).toContain("Repository npm:x:"); + expect(output).toContain("Skipped 1 binary file(s)."); + expect(output).toContain("--cursor 'cursor'"); + expect(output).not.toContain("(no results)"); + }); + it("explains unspecified readiness when it is not an unvisited continuation", () => { + const output = formatGrepText( + result({ targets: [{ ...target, readiness: "UNSPECIFIED" }] }), + ); + expect(output).toContain("Repository npm:x: source readiness unknown."); + }); +}); + +describe("overall grep traversal limitations", () => { + it.each(["NON_RESUMABLE_PARTIAL", "FAILED"] as const)( + "discloses %s even when a sibling cursor remains", + (traversal) => { + for (const hits of [[], [hit]]) { + const output = formatGrepText( + result({ + hits, + totalMatches: hits.length, + traversal, + nextCursor: "sibling cursor", + }), + ); + expect(output).toContain( + "Some requested content could not be searched.", + ); + expect(output).toContain("--cursor 'sibling cursor'"); + expect(output).not.toContain("has no continuation cursor"); + expect(output).not.toContain("(no results)"); + } + }, + ); +}); diff --git a/packages/mcp/src/shared/grep-text-rendering.test.ts b/packages/mcp/src/shared/grep-text-rendering.test.ts index f4dd7358..2a22ec36 100644 --- a/packages/mcp/src/shared/grep-text-rendering.test.ts +++ b/packages/mcp/src/shared/grep-text-rendering.test.ts @@ -120,7 +120,7 @@ describe("grep evidence rendering", () => { ); expect(stripAnsi(colored)).toBe(plain); expect(data).toEqual(before); - expect(plain).toContain("2 matches in 1 line across 1 file"); + expect(plain).toContain("Found 2 matches on 1 line in 1 file."); expect(plain).not.toContain("(2 matches)"); }); it("preserves tab-indented native CRLF-derived source and context rows", () => { @@ -231,7 +231,7 @@ describe("grep evidence rendering", () => { it("counts zero-width matches without fabricated highlight text or empty ANSI spans", () => { const data = page([hit({ matchStartByte: 3, matchEndByte: 3 })]); const text = formatGrepText(data, { useColors: true }); - expect(text).toContain("1 match in 1 line across 1 file"); + expect(text).toContain("Found 1 match on 1 line in 1 file."); expect(text).toContain("3: router"); expect(text).not.toContain(colors.yellow); }); @@ -248,7 +248,7 @@ describe("grep evidence rendering", () => { }); it("leads complete empty pages with the outcome and lists searched sources", () => { expect(formatGrepText(page([]))).toBe( - "No matches.\n\nSources:\n - github:o/r@sha (no results)", + "No matches found.\n\nSources:\n - github:o/r@sha (no results)", ); const text = formatGrepText( page([], { @@ -265,9 +265,7 @@ describe("grep evidence rendering", () => { ], }), ); - expect( - text.startsWith("Zero returned matches; coverage is incomplete."), - ).toBe(true); + expect(text.startsWith("No matches found.")).toBe(true); expect(text).toContain("stale snapshot"); expect(text).toContain( "github:o/r@served (older snapshot, no results on this page)", @@ -288,35 +286,35 @@ describe("grep evidence rendering", () => { nextCursor: cursor, targets: [scope({ traversal: "RESUMABLE_LIMIT" })], }); - const intro = "More matches: repeat this grep, adding:"; + const intro = "Repeat the original grep, adding:"; const cases = [ { syntax: "cli" as const, cursorLine: " --cursor 'opaque cursor'", - header: "# Read files: read --lines $start-$end -- $target $path", + header: "Files: githits read --lines $start-$end -- $target $path", }, { syntax: "mcp" as const, cursorLine: ` cursor=${JSON.stringify(cursor)}`, header: - "# Read files: read target=$target path=$path start_line=$start end_line=$end", + "Files: read target=$target path=$path start_line=$start end_line=$end", }, ] as const; for (const { syntax, cursorLine, header } of cases) { const plain = formatGrepText(data, { syntax, useColors: false }); const colored = formatGrepText(data, { syntax, useColors: true }); - expect(plain.endsWith(`\n${intro}\n${cursorLine}`)).toBe(true); + expect(plain.endsWith(`\n ${intro}\n${cursorLine}`)).toBe(true); const coloredLines = colored.split("\n"); - expect(coloredLines).toContain(`${colors.dim}${header}${colors.reset}`); + expect(coloredLines).toContain(` ${colors.dim}${header}${colors.reset}`); expect(coloredLines.slice(-2)).toEqual([ - `${colors.dim}${intro}${colors.reset}`, - `${colors.dim}${cursorLine}${colors.reset}`, + ` ${intro}`, + ` ${colors.dim}${cursorLine.trimStart()}${colors.reset}`, ]); expect(stripAnsi(colored)).toBe(plain); } - const narrowIntro = ["More matches: repeat", "this grep, adding:"]; + const narrowIntro = [" Repeat the original", " grep, adding:"]; const narrowPlain = formatGrepText(data, { syntax: "cli", useColors: false, @@ -332,8 +330,8 @@ describe("grep evidence rendering", () => { cases[0].cursorLine, ]); expect(narrowColored.split("\n").slice(-(narrowIntro.length + 1))).toEqual([ - ...narrowIntro.map((line) => `${colors.dim}${line}${colors.reset}`), - `${colors.dim}${cases[0].cursorLine}${colors.reset}`, + ...narrowIntro, + ` ${colors.dim}${cases[0].cursorLine.trimStart()}${colors.reset}`, ]); expect(stripAnsi(narrowColored)).toBe(narrowPlain); @@ -344,7 +342,7 @@ describe("grep evidence rendering", () => { expect(noCursor).not.toContain("cursor="); } expect(formatGrepText(page([]))).toBe( - "No matches.\n\nSources:\n - github:o/r@sha (no results)", + "No matches found.\n\nSources:\n - github:o/r@sha (no results)", ); }); }); diff --git a/packages/mcp/src/shared/grep-text.test.ts b/packages/mcp/src/shared/grep-text.test.ts index c08a0067..c8288afd 100644 --- a/packages/mcp/src/shared/grep-text.test.ts +++ b/packages/mcp/src/shared/grep-text.test.ts @@ -72,12 +72,8 @@ describe("grep text formatting", () => { "Sources: - github:expressjs/express@dbac741a - site:expressjs.com (hosted documentation)", ); expect(normalizedText.match(/site:expressjs\.com/g) ?? []).toHaveLength(1); - expect( - lines.filter((line) => line.startsWith("# Read files:")), - ).toHaveLength(1); - expect( - lines.filter((line) => line.startsWith("# Read pages:")), - ).toHaveLength(1); + expect(lines.filter((line) => line.startsWith(" Files:"))).toHaveLength(1); + expect(lines.filter((line) => line.startsWith(" Pages:"))).toHaveLength(1); expect(rendered).not.toContain("Read recipes"); expect(rendered).not.toContain("Hosted page reads"); const expectedRows = new Map(); @@ -149,22 +145,26 @@ describe("grep text formatting", () => { ); const fileRecipe = syntax === "cli" - ? "# Read files: read --lines $start-$end -- $target $path" - : "# Read files: read target=$target path=$path start_line=$start end_line=$end"; + ? "Files: githits read --lines $start-$end -- $target $path" + : "Files: read target=$target path=$path start_line=$start end_line=$end"; const pageRecipe = syntax === "cli" - ? "# Read pages: read --lines $start-$end -- $url" - : "# Read pages: read target=$url start_line=$start end_line=$end"; - const fileRecipeIndex = lines.indexOf(fileRecipe); - const pageRecipeIndex = lines.indexOf(pageRecipe); + ? "Pages: githits read --lines $start-$end -- $url" + : "Pages: read target=$url start_line=$start end_line=$end"; + const fileRecipeIndex = lines.indexOf(` ${fileRecipe}`); + const pageRecipeIndex = lines.indexOf(` ${pageRecipe}`); const continuationIndex = lines.indexOf( - "More matches: repeat this grep, adding:", + " Repeat the original grep, adding:", ); expect(finalContentRowIndex).toBeGreaterThanOrEqual(0); expect(lines[finalContentRowIndex + 1]).toBe(""); - expect(lines.filter((line) => line === fileRecipe)).toHaveLength(1); - expect(lines.filter((line) => line === pageRecipe)).toHaveLength(1); + expect(lines.filter((line) => line === ` ${fileRecipe}`)).toHaveLength( + 1, + ); + expect(lines.filter((line) => line === ` ${pageRecipe}`)).toHaveLength( + 1, + ); expect(fileRecipeIndex).toBeGreaterThan(finalContentRowIndex); expect(pageRecipeIndex).toBeGreaterThan(finalContentRowIndex); expect(fileRecipeIndex).toBeLessThan(pageRecipeIndex); @@ -175,8 +175,8 @@ describe("grep text formatting", () => { { ...result, hits: [], totalMatches: 0 }, { useColors: false, width: 200, syntax }, ); - expect(emptyText).not.toContain("# Read files:"); - expect(emptyText).not.toContain("# Read pages:"); + expect(emptyText).not.toContain("Files:"); + expect(emptyText).not.toContain("Pages:"); } }); @@ -274,7 +274,7 @@ describe("grep text formatting", () => { expect(text).not.toContain("Run grep again"); expect(text).not.toContain("Use the cursor below"); expect(lines.at(-1)).toBe( - `To retry omitted targets, rerun the original query with ${syntax === "cli" ? "--wait 100000" : "wait_timeout_ms=100000"}.`, + ` To retry omitted targets, rerun the original query with ${syntax === "cli" ? "--wait 100000" : "wait_timeout_ms=100000"}.`, ); const cursor = syntax === "cli" ? " --cursor " : " cursor="; expect( @@ -333,7 +333,7 @@ describe("grep text formatting", () => { expect(text).not.toContain("not visited"); expect(text).not.toContain("Omitted:"); expect(text).not.toContain("To retry omitted targets"); - expect(text).toContain("More matches: repeat this grep, adding:"); + expect(text).toContain("Repeat the original grep, adding:"); expect(text).toContain(syntax === "cli" ? " --cursor " : " cursor="); } expect(result).toEqual(before); @@ -490,10 +490,10 @@ describe("grep text formatting", () => { indexingEstimates: [], }; expect(formatGrepText(complete, { syntax })).toContain( - "No matches.\n\nSources:\n - site:expressjs.com (no results, hosted documentation)", + "No matches found.\n\nSources:\n - site:expressjs.com (no results, hosted documentation)", ); expect(formatGrepText({ ...complete, targets: [] }, { syntax })).toBe( - "No matches.", + "No matches found.", ); } }); diff --git a/packages/mcp/src/shared/grep-text.ts b/packages/mcp/src/shared/grep-text.ts index 3e72e9ae..93384057 100644 --- a/packages/mcp/src/shared/grep-text.ts +++ b/packages/mcp/src/shared/grep-text.ts @@ -4,10 +4,15 @@ import type { GrepResult, GrepTargetStatus, } from "@githits/core-internal"; -import { colors, dim, highlightMatch } from "./colors.js"; +import { colors, highlightMatch } from "./colors.js"; import { grepPreparationReason } from "./grep-preparation-text.js"; import { formatPreparationRow } from "./indexing-estimates-text.js"; import { indexingWaitMs } from "./indexing-wait.js"; +import { + appendSearchGrepFooter, + footerAction, + footerProse, +} from "./search-grep-output-text.js"; import { shellQuoteExact } from "./shell-quote.js"; import { formatProvenanceRow, @@ -54,9 +59,8 @@ export function formatGrepText( const groups = groupFiles(result.hits); const omissionsOnly = result.unavailableTargets.length > 0 && - result.targets.every((scope) => !hasCoverageGap(scope)) && - result.traversal !== "FAILED" && - result.traversal !== "CURSOR_EXPIRED"; + result.targets.every((scope) => !hasPageCoverageGap(scope)) && + !["FAILED", "CURSOR_EXPIRED"].includes(result.traversal); const retryableOmissionsOnly = omissionsOnly && result.unavailableTargets.every((target) => target.retryable); @@ -70,14 +74,22 @@ export function formatGrepText( : kinds.has("GrepSiteHit") ? `page${groups.length === 1 ? "" : "s"}` : `file${groups.length === 1 ? "" : "s"}`; + const pageGap = + result.targets.some(hasPageCoverageGap) || + (!omissionsOnly && + !["COMPLETE", "RESUMABLE_LIMIT"].includes(result.traversal)); prose( - result.hits.length === 0 - ? isExhaustive(result) - ? "No matches." + result.hits.length + ? `Found ${result.totalMatches} match${result.totalMatches === 1 ? "" : "es"} on ${matchingLines} line${matchingLines === 1 ? "" : "s"} in ${groups.length} ${noun}.` + : result.nextCursor && + !pageGap && + (result.unavailableTargets.length === 0 || retryableOmissionsOnly) + ? retryableOmissionsOnly + ? "No matches available yet on this page." + : "No matches on this page." : retryableOmissionsOnly - ? "No matches yet." - : "Zero returned matches; coverage is incomplete." - : `${result.totalMatches} match${result.totalMatches === 1 ? "" : "es"} in ${matchingLines} line${matchingLines === 1 ? "" : "s"} across ${groups.length} ${noun}${result.nextCursor ? "; more available" : ""}`, + ? "No matches available yet." + : "No matches found.", ); if (options.useColors) lines[0] = `${colors.bold}${lines[0]}${colors.reset}`; if (result.targets.length) { @@ -156,10 +168,14 @@ export function formatGrepText( ); else if ( result.traversal !== "COMPLETE" && - !result.nextCursor && + (!result.nextCursor || result.traversal !== "RESUMABLE_LIMIT") && !omissionsOnly ) - prose("Traversal is incomplete and has no continuation cursor."); + prose( + result.nextCursor + ? "Some requested content could not be searched." + : "Traversal is incomplete and has no continuation cursor.", + ); for (const [index, group] of groups.entries()) { const first = group.first; const target = first.read.target; @@ -192,49 +208,53 @@ export function formatGrepText( "Safety normalization applied; physical source coordinates remain in JSON.", ); } + const read: string[] = []; + const more: string[] = []; + const followUp: string[] = []; + const proseLines = (value: string): string[] => + footerProse(escapeText(value), options.width ?? 80); + const operand = (value: string): string => + footerAction(value, options.useColors === true); if (groups.length) { - lines.push(""); if (kinds.has("GrepRepositoryHit")) - lines.push( - dim( + read.push( + operand( options.syntax === "mcp" - ? "# Read files: read target=$target path=$path start_line=$start end_line=$end" - : "# Read files: read --lines $start-$end -- $target $path", - options.useColors === true, + ? "Files: read target=$target path=$path start_line=$start end_line=$end" + : "Files: githits read --lines $start-$end -- $target $path", ), ); if (kinds.has("GrepSiteHit")) - lines.push( - dim( + read.push( + operand( options.syntax === "mcp" - ? "# Read pages: read target=$url start_line=$start end_line=$end" - : "# Read pages: read --lines $start-$end -- $url", - options.useColors === true, + ? "Pages: read target=$url start_line=$start end_line=$end" + : "Pages: githits read --lines $start-$end -- $url", ), ); } - if (result.nextCursor) { - const footerLines = [ - ...wrap( - escapeText("More matches: repeat this grep, adding:"), - options.width ?? 80, + if (result.nextCursor) + more.push( + ...proseLines("Repeat the original grep, adding:"), + operand( + options.syntax === "mcp" + ? `cursor=${JSON.stringify(result.nextCursor)}` + : `--cursor ${shellQuoteExact(result.nextCursor)}`, ), - options.syntax === "mcp" - ? ` cursor=${JSON.stringify(result.nextCursor)}` - : ` --cursor ${shellQuoteExact(result.nextCursor)}`, - ]; - lines.push( - "", - ...footerLines.map((line) => dim(line, options.useColors === true)), ); - } if (result.unavailableTargets.some((target) => target.retryable)) { const wait = indexingWaitMs(result.indexingEstimates); - lines.push(""); - prose( - `To retry omitted targets, rerun the original query with ${options.syntax === "mcp" ? `wait_timeout_ms=${wait}` : `--wait ${wait}`}.`, + followUp.push( + ...proseLines( + `To retry omitted targets, rerun the original query with ${options.syntax === "mcp" ? `wait_timeout_ms=${wait}` : `--wait ${wait}`}.`, + ), ); } + appendSearchGrepFooter( + lines, + { read, more, followUp }, + options.useColors === true, + ); return lines.join("\n"); } @@ -244,7 +264,12 @@ function renderCoverage( ): void { const prefix = `${scope.kind === "REPOSITORY" ? "Repository" : "Hosted docs"} ${scope.target}`; const notes: string[] = []; - if (scope.readiness !== "UNSPECIFIED" && scope.readiness !== "CURRENT") + if ( + scope.readiness === "UNSPECIFIED" && + scope.traversal !== "RESUMABLE_LIMIT" + ) + notes.push("source readiness unknown"); + else if (scope.readiness !== "UNSPECIFIED" && scope.readiness !== "CURRENT") notes.push(readinessNote(scope)); if (scope.traversal !== "COMPLETE" && scope.traversal !== "RESUMABLE_LIMIT") notes.push(traversalNote(scope)); @@ -331,10 +356,15 @@ function isExhaustive(result: GrepResult): boolean { result.targets.every((scope) => !hasCoverageGap(scope)) ); } -function hasCoverageGap(scope: GrepTargetStatus): boolean { +/** Resumable, unvisited scopes are pagination; errors and skips remain gaps. */ +function hasPageCoverageGap(scope: GrepTargetStatus): boolean { return ( - scope.readiness !== "CURRENT" || - scope.traversal !== "COMPLETE" || + (scope.readiness !== "CURRENT" && + !( + scope.readiness === "UNSPECIFIED" && + scope.traversal === "RESUMABLE_LIMIT" + )) || + !["COMPLETE", "RESUMABLE_LIMIT"].includes(scope.traversal) || scope.errorCode !== null || Boolean( scope.binaryFilesSkipped || @@ -344,6 +374,14 @@ function hasCoverageGap(scope: GrepTargetStatus): boolean { ) ); } +function hasCoverageGap(scope: GrepTargetStatus): boolean { + return ( + scope.readiness !== "CURRENT" || + scope.traversal !== "COMPLETE" || + hasPageCoverageGap(scope) + ); +} + function formatSources( scopes: GrepTargetStatus[], matchedScopes: Set, diff --git a/packages/mcp/src/shared/indexing-estimates.test.ts b/packages/mcp/src/shared/indexing-estimates.test.ts index d9c95782..6595043e 100644 --- a/packages/mcp/src/shared/indexing-estimates.test.ts +++ b/packages/mcp/src/shared/indexing-estimates.test.ts @@ -57,7 +57,7 @@ describe("uniform indexing evidence presentation", () => { const result = pending(); const before = structuredClone(result); const output = formatGrepText(result, { syntax, width: 160 }); - expect(output).toContain("No matches yet."); + expect(output).toContain("No matches available yet."); expect(output).toContain("(indexing, estimated total: 38-57s"); expect(output).toContain("time spent indexing: 90s"); expect(output).toContain("To retry omitted targets"); @@ -97,7 +97,7 @@ describe("uniform indexing evidence presentation", () => { indexingEstimates: [], }); const output = formatGrepText(result); - expect(output).toContain("coverage is incomplete"); + expect(output).toContain("Cursor expired"); expect(output).toContain("Cursor expired"); expect(output).toContain("unknown_reason"); expect(output).not.toContain("To retry omitted targets"); @@ -182,7 +182,7 @@ describe("uniform indexing evidence presentation", () => { indexingEstimates: [], }); const text = formatGrepText(result); - expect(text).toContain("coverage is incomplete"); + expect(text).toContain("No matches found."); expect(text).toContain("Omitted:\n - npm:x"); expect(text).not.toContain("Traversal is incomplete"); expect(text).not.toContain("No matches yet"); diff --git a/packages/mcp/src/shared/search-grep-output-text.ts b/packages/mcp/src/shared/search-grep-output-text.ts new file mode 100644 index 00000000..375b88b7 --- /dev/null +++ b/packages/mcp/src/shared/search-grep-output-text.ts @@ -0,0 +1,38 @@ +import { colors, dim } from "./colors.js"; +import { wrapTerminalProse } from "./terminal-text.js"; + +export interface SearchGrepFooter { + read?: readonly string[]; + more?: readonly string[]; + followUp?: readonly string[]; +} + +/** Shared fixed footer anatomy; tools own the meaning and exact action operands. */ +export function appendSearchGrepFooter( + lines: string[], + footer: SearchGrepFooter, + useColors: boolean, +): void { + for (const [label, body] of [ + ["Read:", footer.read], + ["More results:", footer.more], + ["Follow-up:", footer.followUp], + ] as const) { + if (!body?.length) continue; + lines.push( + "", + useColors ? `${colors.bold}${label}${colors.reset}` : label, + ...body.map((line) => ` ${line}`), + ); + } +} + +/** Fixed operands bypass prose wrapping and retain identical plain text. */ +export function footerAction(value: string, useColors: boolean): string { + return dim(value, useColors); +} + +/** Reserve the footer's two-column indent while wrapping only authored prose. */ +export function footerProse(value: string, width: number): string[] { + return wrapTerminalProse(value, Math.max(20, width) - 2); +} diff --git a/packages/mcp/src/shared/unified-search-presentation.test.ts b/packages/mcp/src/shared/unified-search-presentation.test.ts index 2f669049..8d6d2f69 100644 --- a/packages/mcp/src/shared/unified-search-presentation.test.ts +++ b/packages/mcp/src/shared/unified-search-presentation.test.ts @@ -195,6 +195,7 @@ describe("projectUnifiedSearchPresentation", () => { expect(presentation.availability).toEqual({ kind: "final", hasSnapshot: true, + partialResults: false, resultCount: 1, }); expect(presentation.lifecycle).toEqual({ @@ -242,6 +243,7 @@ describe("projectUnifiedSearchPresentation", () => { expect(presentation.availability).toEqual({ kind: "empty", hasSnapshot: true, + partialResults: false, resultCount: 0, }); expect(groupedSources(presentation)).toEqual([ @@ -649,6 +651,7 @@ describe("projectUnifiedSearchPresentation", () => { expect(presentation.availability).toEqual({ kind, hasSnapshot: true, + partialResults, resultCount: 1, }); }, @@ -678,6 +681,7 @@ describe("projectUnifiedSearchPresentation", () => { expect(presentation.availability).toEqual({ kind: "no_snapshot", hasSnapshot: false, + partialResults: false, resultCount: 0, }); expect(groupedSources(presentation)).toEqual([]); diff --git a/packages/mcp/src/shared/unified-search-presentation.ts b/packages/mcp/src/shared/unified-search-presentation.ts index ac14c108..3af08aae 100644 --- a/packages/mcp/src/shared/unified-search-presentation.ts +++ b/packages/mcp/src/shared/unified-search-presentation.ts @@ -39,6 +39,8 @@ export interface UnifiedSearchAvailability { kind: UnifiedSearchAvailabilityKind; hasSnapshot: boolean; resultCount: number; + /** Backend subset truth survives even an empty snapshot; text-only facts. */ + partialResults: boolean; } export type UnifiedSearchActiveStatus = "PENDING" | "INDEXING" | "SEARCHING"; @@ -429,7 +431,12 @@ function projectAvailability( lifecycle: UnifiedSearchLifecycle, ): UnifiedSearchAvailability { if (!snapshot) { - return { kind: "no_snapshot", hasSnapshot: false, resultCount: 0 }; + return { + kind: "no_snapshot", + hasSnapshot: false, + resultCount: 0, + partialResults: false, + }; } const resultCount = snapshot.results.length; const kind = @@ -440,7 +447,12 @@ function projectAvailability( : lifecycle.kind === "active" ? "interim" : "final"; - return { kind, hasSnapshot: true, resultCount }; + return { + kind, + hasSnapshot: true, + resultCount, + partialResults: snapshot.partialResults, + }; } function projectSources( diff --git a/packages/mcp/src/shared/unified-search-semantic-text.test.ts b/packages/mcp/src/shared/unified-search-semantic-text.test.ts index fa3ff52c..941ea0ae 100644 --- a/packages/mcp/src/shared/unified-search-semantic-text.test.ts +++ b/packages/mcp/src/shared/unified-search-semantic-text.test.ts @@ -178,7 +178,7 @@ describe("semantic search text", () => { expect(render(hit)).toContain( "github:owner/monorepo@main packages/pkg/src/client.ts:142-145", ); - expect(render(hit)).not.toContain("read target="); + expect(render(hit).split("\n\nRead:")[0]).not.toContain("read target="); }); it.each(["github:owner/monorepo@main", "owner/monorepo@main"])( @@ -200,7 +200,7 @@ describe("semantic search text", () => { expect(text).toContain( `${targetLabel} packages/pkg/src/client.ts:142-145`, ); - expect(text).not.toContain("read target="); + expect(text.split("\n\nRead:")[0]).not.toContain("read target="); expect(text).not.toContain("npm:pkg"); expect(text).not.toContain("#main"); }, @@ -290,7 +290,7 @@ describe("v31 search presentation", () => { expect(text).not.toContain("send"); expect(text).not.toContain("return response"); expect(text).not.toContain("Snippet unavailable"); - expect(text.split("\n")).toHaveLength(3); + expect(text.split("\n\nRead:")[0]!.split("\n")).toHaveLength(3); hit.type = "repository_doc"; hit.title = "Arbitrary heading"; expect(render(hit)).toContain( diff --git a/packages/mcp/src/shared/unified-search-snapshot-text.test.ts b/packages/mcp/src/shared/unified-search-snapshot-text.test.ts index 073d6067..c9b36e22 100644 --- a/packages/mcp/src/shared/unified-search-snapshot-text.test.ts +++ b/packages/mcp/src/shared/unified-search-snapshot-text.test.ts @@ -144,7 +144,7 @@ describe("snapshot search text received by agents", () => { expect(text.match(/committed \d{4}-\d{2}-\d{2}/g) ?? []).toHaveLength( Number(Boolean(servedDate)) + Number(Boolean(requestedDate)), ); - expect(text).toContain("Next: use these hits now; read for details:"); + expect(text).toContain("Use these results now; example read:"); expect(text).toContain("If you need current HEAD"); expect(text).not.toContain("T23:"); expect(text).not.toContain("unknown date"); @@ -177,7 +177,7 @@ describe("snapshot search text received by agents", () => { expect(sourceSection).not.toContain("2026-10-05"); expect(text).not.toContain("different commit"); expect(text).not.toContain("If you need current HEAD"); - expect(text).toContain("Next: use these hits now"); + expect(text).toContain("Use these results now"); if (servedDate) expect(text).toContain("committed 2026-09-01"); else expect(sourceSection).not.toContain("committed"); } @@ -219,10 +219,10 @@ describe("snapshot search text received by agents", () => { "github:anomalyco/opencode@0112a92c", ); expect(text.replace(/\s+/g, " ")).toContain("observed HEAD"); - expect(text).toContain("next_offset=3"); - expect(text).toContain("1 result"); + expect(text).toContain(syntax === "mcp" ? "offset=3" : "--offset 3"); + expect(text).toContain("Found 1"); expect(text).not.toContain("partial result"); - expect(text).toContain("Next: use these hits now; read for details:"); + expect(text).toContain("Use these results now; example read:"); const read = syntax === "mcp" ? `read target="github:anomalyco/opencode@bbd72fb8" path="${path}" start_line=480 end_line=490` @@ -236,8 +236,8 @@ describe("snapshot search text received by agents", () => { ? 'search_status search_ref="recorded-search" wait_timeout_ms=120000' : "githits search-status recorded-search --wait 120", ); - expect(text).not.toContain("Next: search_status"); - expect(text).not.toContain("Next: githits search-status"); + expect(text).not.toContain("Follow-up:\n search_status"); + expect(text).not.toContain("Follow-up:\n githits search-status"); expect(text).toContain( "For a specific ref, search github:anomalyco/opencode@.", ); @@ -278,9 +278,9 @@ describe("snapshot search text received by agents", () => { const payload = snapshot(); payload.results[0]!.readTarget = undefined; for (const text of both(payload)) { - expect(text).toContain("Next: use these hits now."); + expect(text).toContain("Use these results now."); expect(text).not.toContain("read for details:"); - expect(text).not.toContain("read target="); + expect(text.split("\n\nRead:")[0]).not.toContain("read target="); expect(text).toContain("If you need current HEAD"); } }); @@ -370,11 +370,11 @@ describe("snapshot search text received by agents", () => { payload.results = []; resolution(payload).served!.committedAt = "2026-09-01T00:00:00Z"; for (const text of both(payload)) { - expect(text).toContain("No results yet"); + expect(text).toContain("No results on this page."); expect(text).toContain( - 'Next: search_status search_ref="recorded-search" wait_timeout_ms=120000', + 'search_status search_ref="recorded-search" wait_timeout_ms=120000', ); - expect(text).not.toContain("read target="); + expect(text.split("\n\nRead:")[0]).not.toContain("read target="); expect(text).not.toContain("Sources:"); expect(text).not.toContain("older snapshot"); } @@ -403,13 +403,12 @@ describe("snapshot search text received by agents", () => { : "(indexing when observed, estimated", ); expect(text).toContain("committed 2026-09-01"); - expect(text).not.toContain("Next: use these hits"); - expect(text).not.toContain("read target="); + expect(text).not.toContain("Use these results"); + expect(text.split("\n\nRead:")[0]).not.toContain("read target="); expect(text).not.toContain("If you need current HEAD"); - if (status === "INDEXING") - expect(text).toContain("Next: search_status"); + if (status === "INDEXING") expect(text).toContain("search_status"); else { - expect(text).toContain("search again later"); + expect(text).toContain("Search again later"); expect(text).not.toContain("search_status"); } } @@ -483,7 +482,7 @@ describe("snapshot search text received by agents", () => { ).toHaveLength(0); } expect(text).not.toContain("If you need current HEAD"); - expect(text).toContain("Next: use these hits"); + expect(text).toContain("Use these results"); expect(text).toContain(explicitTarget); } }, @@ -501,10 +500,11 @@ describe("snapshot search text received by agents", () => { resultCount: 0, }); for (const text of both(payload)) { - expect(text).toContain("1 partial result"); - expect(text).toContain("1/2 ready"); + expect(text).toContain("Found 1 code result."); + expect(text).not.toContain("1/2 ready"); + expect(text).toContain("site:example.com docs"); expect(text).toContain("indexing: site:example.com docs"); - expect(text).toContain("Next: use these hits"); + expect(text).toContain("Use these results"); } }); @@ -515,7 +515,7 @@ describe("snapshot search text received by agents", () => { payload.progress!.status = status; resolution(payload).served!.committedAt = "2026-09-01T00:00:00Z"; for (const text of both(payload)) { - expect(text).toContain("Next: use these hits"); + expect(text).toContain("Use these results"); expect(text).toContain("committed 2026-09-01"); expect(text).toContain("search again"); expect(text).not.toContain("search_status"); @@ -549,7 +549,7 @@ describe("snapshot search text received by agents", () => { const prep = flat.split("Preparing:")[1]!.split("Requested:")[0]!; expect(prep).not.toContain("observed HEAD"); expect(prep).not.toContain("committed"); - expect(text).toContain("Next: use these hits now"); + expect(text).toContain("Use these results now"); } }); it("shared rows never join requested dates or HEAD across raw repository identities", () => { @@ -608,7 +608,7 @@ describe("requested indexing explanation without matching estimates", () => { expect(flat).toContain("Requested ref is being indexed."); expect(text).not.toContain("requested_ref_indexing"); expect(text).not.toContain("\nHEAD)"); - expect(text).toContain("Next: use these hits now"); + expect(text).toContain("Use these results now"); if (estimates === "unmatched") { expect(flat).toContain( "github:anomalyco/opencode@cccccccc (indexing, estimated total: 100-120s)", diff --git a/packages/mcp/src/shared/unified-search-status-text.test.ts b/packages/mcp/src/shared/unified-search-status-text.test.ts index f9d6cfd0..8701b673 100644 --- a/packages/mcp/src/shared/unified-search-status-text.test.ts +++ b/packages/mcp/src/shared/unified-search-status-text.test.ts @@ -56,9 +56,7 @@ describe("renderUnifiedSearchStatusText", () => { }); const text = renderUnifiedSearchStatusText(payload); - expect(firstLine(text)).toBe( - "1 result | 1 docs page | indexing | 0/1 ready", - ); + expect(firstLine(text)).toBe("Found 1 documentation result."); expect(text).toContain( "express/routing [docs page] npm:express - source URL unavailable - Routing", ); @@ -75,9 +73,7 @@ describe("renderUnifiedSearchStatusText", () => { result: result({ partialResults: true, results: [hit()] }), }), ); - expect(firstLine(statusText)).toBe( - "1 partial result | 1 docs page | indexing | 0/1 ready", - ); + expect(firstLine(statusText)).toBe("Found 1 documentation result."); }); it("distinguishes status snapshots with partialResults true", () => { @@ -85,8 +81,8 @@ describe("renderUnifiedSearchStatusText", () => { result: result({ partialResults: true, results: [hit()] }), }); const text = renderUnifiedSearchStatusText(payload); - expect(firstLine(text)).toContain("1 partial result"); - expect(text).not.toContain("1 interim result"); + expect(firstLine(text)).toContain("Found 1 documentation result."); + expect(text).not.toContain("interim"); }); it.each([false, true])( @@ -118,13 +114,13 @@ describe("renderUnifiedSearchStatusText", () => { actionSyntax: "cli", }); expect(mcp).toContain("[1] opaque-page [docs page]"); - expect(mcp.includes("read target=")).toBe(!completed); - expect(cli.includes("githits read ")).toBe(!completed); - expect(mcp).toContain("next_offset=5"); - expect(cli).toContain("next_offset=5"); + expect(mcp.includes("read target=")).toBe(true); + expect(cli.includes("githits read ")).toBe(true); + expect(mcp).toContain("offset=5"); + expect(cli).toContain("--offset 5"); if (!completed) { - expect(mcp).toContain("Next: use these hits"); - expect(cli).toContain("Next: use these hits"); + expect(mcp).toContain("Use these results"); + expect(cli).toContain("Use these results"); } }, ); @@ -236,9 +232,7 @@ describe("renderUnifiedSearchStatusText", () => { }, }), ); - expect(firstLine(text)).toBe( - "No result snapshot yet | preparing | 0/1 ready", - ); + expect(firstLine(text)).toBe("No results available yet."); expect(text).not.toContain("Indexing:"); expect(text).not.toContain("No hits"); expect(text).toContain("- npm:express"); @@ -262,12 +256,12 @@ describe("renderUnifiedSearchStatusText", () => { }), }; const text = renderUnifiedSearchStatusText(payload); - expect(firstLine(text)).toBe("No results"); + expect(firstLine(text)).toBe("No results found."); expect(text).toContain( "Sources:\n - npm:express@5.2.1 (code, no results)", ); expect(text).toContain( - 'Next: shorten or broaden query; use source="symbol"; use grep.', + 'Try: shorten or broaden query; use source="symbol"; use grep.', ); expect(text).not.toContain("Search search-ref-empty | completed"); }); @@ -317,7 +311,7 @@ describe("renderUnifiedSearchStatusText", () => { expect(text).toContain("Fix: verify public repository/ref."); expect(text).toContain("Fix: verify site host/path."); expect(text).toContain("Fix: verify or replace target."); - expect(text).not.toContain("search again later"); + expect(text).not.toContain("Search again later"); expect(text).not.toContain("searchRef="); }); @@ -331,7 +325,7 @@ describe("renderUnifiedSearchStatusText", () => { }), }; const text = renderUnifiedSearchStatusText(payload); - expect(firstLine(text)).toContain("1 result"); + expect(firstLine(text)).toContain("Found 1"); expect(text).not.toContain("Search search-ref-evidence | completed"); expect(text).toContain("For updated results, search again."); expect(text).not.toContain("opaque backend notice"); @@ -352,12 +346,10 @@ describe("renderUnifiedSearchStatusText", () => { }, }), ); - expect(firstLine(text)).toBe( - `No result snapshot | ${status.toLowerCase()} | 0/1 ready`, - ); - expect(text).toContain("Next: search again later."); + expect(firstLine(text)).toBe("No result snapshot available."); + expect(text).toContain("Search again later."); expect(text).not.toContain("Do not poll"); - expect(text).not.toContain("Next: search_status"); + expect(text).not.toContain("search_status"); }, ); @@ -372,12 +364,10 @@ describe("renderUnifiedSearchStatusText", () => { }, }), ); - expect(firstLine(text)).toBe( - "No result snapshot | status unknown | 0/1 ready", - ); - expect(text).toContain("Next: search again later."); + expect(firstLine(text)).toBe("No result snapshot available."); + expect(text).toContain("Search again later."); expect(text).not.toContain("Do not poll"); - expect(text).not.toContain("Next: search_status"); + expect(text).not.toContain("search_status"); }); }); @@ -410,9 +400,7 @@ describe("search preparation sections", () => { }), { width: 60 }, ); - expect(text.split("\n")[0]).toContain( - status.toLowerCase() === "pending" ? "preparing" : status.toLowerCase(), - ); + expect(text.split("\n")[0]).toMatch(/^Found 1 .* result\.$/); expect(text).toContain("\n\nPreparing:\n - "); const section = text.split("Preparing:\n")[1]!.split("\n\n")[0]!; const lines = section.split("\n"); @@ -421,7 +409,7 @@ describe("search preparation sections", () => { expect(lines.every((line) => line.length <= 60)).toBe(true); expect(section).toContain("indexing"); expect(section).toContain("preparing documentation"); - expect(text).toContain("1 result"); + expect(text).toContain("Found 1"); expect(text).toContain("search_status"); expect(text).not.toContain("remaining ETA"); }, diff --git a/packages/mcp/src/shared/unified-search-text.test.ts b/packages/mcp/src/shared/unified-search-text.test.ts index dd2b97c6..17580568 100644 --- a/packages/mcp/src/shared/unified-search-text.test.ts +++ b/packages/mcp/src/shared/unified-search-text.test.ts @@ -314,7 +314,7 @@ describe("renderUnifiedSearchSuccess", () => { ); expect(text.split("\n")[0]).toBe( - "10 results | 5 repo docs, 5 docs pages | next_offset=10", + "Found 5 repository documentation results and 5 documentation results.", ); expect(text).toContain( "Sources:\n - site:expressjs.com (hosted documentation, requested: npm:express@5.2.1)", @@ -331,9 +331,9 @@ describe("renderUnifiedSearchSuccess", () => { ); expect(text).not.toContain("githits docs read"); expect(text).not.toContain("docs_read"); - expect(text).not.toContain(" read target="); + expect(text.split("\n\nRead:")[0]).not.toContain(" read target="); expect(text).not.toContain("### router.use()"); - expect(text.match(/next_offset=10/g)).toHaveLength(1); + expect(text.match(/offset=10/g)).toHaveLength(1); expect(text.length).toBeLessThan(3459); }); @@ -476,7 +476,7 @@ describe("renderUnifiedSearchSuccess", () => { expect(render()).toContain( "src/auth.ts:17-27 [repo code, candidate; visible terms: auth, session, store] - interface AuthSessionStore", ); - expect(render().split("\n")).toHaveLength(3); + expect(render().split("\n\nRead:")[0]!.split("\n")).toHaveLength(3); hit.locator.symbolContext!.definitionRange!.filePath = "src/other.ts"; hit.locator.symbolContext!.definitionRange!.repositoryFilePath = "src/other.ts"; @@ -794,8 +794,8 @@ describe("renderUnifiedSearchSuccess", () => { expect(text).toContain( "[2] npm:githits@0.22.1 docs/implementation/config.md:69-79 [repo doc, candidate] - Local Storage", ); - expect(text).not.toContain(" githits read "); - expect(text).not.toContain(" read target="); + expect(text.split("\n\nRead:")[0]).not.toContain(" githits read "); + expect(text.split("\n\nRead:")[0]).not.toContain(" read target="); } }); @@ -826,7 +826,7 @@ describe("renderUnifiedSearchSuccess", () => { ]), ); expect(text).toContain("[1] npm:pkg@1.2.3 docs/auth.md:42-52 [repo doc]"); - expect(text).not.toContain(" read target="); + expect(text.split("\n\nRead:")[0]).not.toContain(" read target="); }); it("keeps repo-doc producer evidence while omitting its backend action", () => { @@ -858,7 +858,7 @@ describe("renderUnifiedSearchSuccess", () => { expect(text).toContain( "[1] pypi:flask@3.1.3 docs/design.rst:83-93 [repo doc] - design.rst", ); - expect(text).not.toContain(" read target="); + expect(text.split("\n\nRead:")[0]).not.toContain(" read target="); }); it("does not promote repository documentation with associated symbol metadata", () => { @@ -1069,7 +1069,7 @@ describe("renderUnifiedSearchSuccess", () => { it("starts completed hits with the outcome and preserves hit anatomy", () => { const text = renderUnifiedSearchSuccess(completed([codeHit()])); - expect(firstLine(text)).toContain("1 result"); + expect(firstLine(text)).toContain("Found 1"); expect(firstLine(text)).not.toContain("search |"); expect(text).toContain( "[1] cline/cline@v3.4.2 src/integrations/diff/strategies/multi-search-replace.ts:142-156 [repo code] -\n applyEdit", @@ -1090,8 +1090,8 @@ describe("renderUnifiedSearchSuccess", () => { { results: payload.results }, ); - expect(text).toBe( - "1 result | 1 repo code hit\n\n" + + expect(text.split("\n\nRead:")[0]).toBe( + "Found 1 code result.\n\n" + "[1] cline/cline@v3.4.2 src/integrations/diff/strategies/multi-search-replace.ts:142-156 [repo code] -\n" + " applyEdit\n" + " Snippet unavailable", @@ -1118,10 +1118,12 @@ describe("renderUnifiedSearchSuccess", () => { ); const docsText = renderUnifiedSearchSuccess(completed([docsHit()])); - expect(firstLine(repoText)).toBe("1 result | 1 repo doc"); - expect(firstLine(docsText)).toBe("1 result | 1 docs page"); + expect(firstLine(repoText)).toBe( + "Found 1 repository documentation result.", + ); + expect(firstLine(docsText)).toBe("Found 1 documentation result."); expect(firstLine(renderUnifiedSearchSuccess(completed([codeHit()])))).toBe( - "1 result | 1 repo code hit", + "Found 1 code result.", ); expect( firstLine( @@ -1129,7 +1131,7 @@ describe("renderUnifiedSearchSuccess", () => { completed([codeHit({ type: "repository_symbol" })]), ), ), - ).toBe("1 result | 1 repo symbol"); + ).toBe("Found 1 symbol result."); }); it("uses ASCII separators without changing Unicode payload text", () => { @@ -1242,7 +1244,7 @@ describe("renderUnifiedSearchSuccess", () => { `[1] ${docsReadTarget} [docs page] aider-AI/aider - aider.chat/docs/more/edit-formats.html -`, ); expect(text).not.toContain("[1] aider/edit-formats [docs page]"); - expect(text).not.toContain(" read target="); + expect(text.split("\n\nRead:")[0]).not.toContain(" read target="); }); it("keeps a hosted source fragment as provenance and selects the heading explicitly", () => { @@ -1267,7 +1269,7 @@ describe("renderUnifiedSearchSuccess", () => { expect(text).toContain( `[1] ${docsReadTarget} [docs page] npm:express - #route-handlers -`, ); - expect(text).not.toContain(" read target="); + expect(text.split("\n\nRead:")[0]).not.toContain(" read target="); expect(text).not.toContain("start_line="); expect(text).not.toContain("end_line="); }); @@ -1459,12 +1461,12 @@ describe("renderUnifiedSearchSuccess", () => { }), ); - expect(firstLine(text)).toBe("No results"); + expect(firstLine(text)).toBe("No results found."); expect(text).toContain( "Sources:\n - npm:express@5.2.1 (code, no results)", ); - expect(text).toContain( - 'Next: shorten or broaden query; remove restrictive filters; use source="symbol"; use grep.', + expect(text.replace(/\s+/g, " ")).toContain( + 'Try: shorten or broaden query; remove restrictive filters; use source="symbol"; use grep.', ); expect(text).not.toContain('query="'); expect(text).not.toContain("Do not repeat"); @@ -1570,7 +1572,7 @@ describe("renderUnifiedSearchSuccess", () => { expect(text).toContain("Fix: verify public repository/ref."); expect(text).toContain("Fix: verify site host/path."); expect(text).toContain("Fix: verify or replace target."); - expect(text).not.toContain("search again later"); + expect(text).not.toContain("Search again later"); expect(text).not.toContain("searchRef"); expect(text.match(/Fix:/g)).toHaveLength(4); @@ -1635,7 +1637,7 @@ describe("renderUnifiedSearchSuccess", () => { ); expect(text).toBe( - "No results\n\n" + + "No results found.\n\n" + "- npm:express latest\n" + " package unresolved: code\n" + " Try: npm:express@5.1.0", @@ -1682,23 +1684,23 @@ describe("renderUnifiedSearchSuccess", () => { }, ); - it("renders the supplied n8n active empty snapshot with one concise readiness block", () => { + it("renders the supplied n8n active empty snapshot with one concise target block", () => { const text = renderUnifiedSearchSuccess(n8nActiveEmpty()); expect(text).toBe( - "No results yet | indexing | 0/1 ready\n\n" + + "No results available yet.\n\n" + "- npm:n8n -> 2.36.7\n" + " indexing: code, repository docs; available: n8n.io docs (1,480 pages; capped);\n" + - " indexed: versions 2.26.9, 2.26.5, 2.23.2 +2, refs HEAD, master\n\n" + - 'Next: search_status search_ref="fabUr1S3MEVeSgD93pMoSQ" wait_timeout_ms=30000', + " indexed: versions 2.26.9, 2.26.5, 2.23.2 (+2 more), refs HEAD, master\n\n" + + 'Follow-up:\n search_status search_ref="fabUr1S3MEVeSgD93pMoSQ" wait_timeout_ms=30000', ); expect(text).not.toContain("Do not repeat"); expect(text).not.toContain("indexingRef"); expect(text).not.toContain("freshnessReason"); expect(text).not.toContain("Opaque evidence notice"); - expect(text.match(/indexing/g)).toHaveLength(2); + expect(text.match(/indexing/g)).toHaveLength(1); expect(text.match(/available:/g)).toHaveLength(1); - expect(text.match(/Next:/g)).toHaveLength(1); + expect(text.match(/Follow-up:/g)).toHaveLength(1); }); it("keeps one hit layout while rendering surface-native status commands", () => { @@ -1707,17 +1709,17 @@ describe("renderUnifiedSearchSuccess", () => { const cli = renderUnifiedSearchSuccess(payload, { actionSyntax: "cli" }); expect(cli).toContain( - "Next: githits search-status fabUr1S3MEVeSgD93pMoSQ --wait 30", + "githits search-status fabUr1S3MEVeSgD93pMoSQ --wait 30", ); expect(cli).not.toContain("search_status search_ref="); expect( cli.replace( - "Next: githits search-status fabUr1S3MEVeSgD93pMoSQ --wait 30", + "githits search-status fabUr1S3MEVeSgD93pMoSQ --wait 30", "Next: ", ), ).toBe( mcp.replace( - 'Next: search_status search_ref="fabUr1S3MEVeSgD93pMoSQ" wait_timeout_ms=30000', + 'search_status search_ref="fabUr1S3MEVeSgD93pMoSQ" wait_timeout_ms=30000', "Next: ", ), ); @@ -1729,14 +1731,16 @@ describe("renderUnifiedSearchSuccess", () => { "[1] cline/cline@v3.4.2 src/integrations/diff/strategies/multi-search-replace.ts:142-156 [repo code] -\n applyEdit", ); const mcpCode = renderUnifiedSearchSuccess(completed([codeHit()])); - expect(code).toBe(mcpCode); - expect(code).not.toContain(" githits read "); - expect(mcpCode).not.toContain(" read target="); + expect(code.split("\n\nRead:")[0]).toBe(mcpCode.split("\n\nRead:")[0]); + expect(code).toContain("githits read "); + expect(mcpCode).toContain("read target="); + expect(code.split("\n\nRead:")[0]).not.toContain(" githits read "); + expect(mcpCode.split("\n\nRead:")[0]).not.toContain(" read target="); const narrowCode = renderUnifiedSearchSuccess(completed([codeHit()]), { actionSyntax: "cli", width: 40, }); - expect(narrowCode).not.toContain(" githits read "); + expect(narrowCode.split("\n\nRead:")[0]).not.toContain(" githits read "); const repositoryCode = renderUnifiedSearchSuccess( completed([ @@ -1762,7 +1766,9 @@ describe("renderUnifiedSearchSuccess", () => { expect(repositoryCode).toContain( "[1] github:cline/cline@main src/index.ts:10-20 [repo code] - applyEdit", ); - expect(repositoryCode).not.toContain(" githits read "); + expect(repositoryCode.split("\n\nRead:")[0]).not.toContain( + " githits read ", + ); const docs = renderUnifiedSearchSuccess(completed([docsHit()]), { actionSyntax: "cli", @@ -1770,10 +1776,10 @@ describe("renderUnifiedSearchSuccess", () => { expect(docs).toContain( "[1] https://aider.chat/docs/more/edit-formats.html [docs page] aider-AI/aider -\n Edit Formats", ); - expect(docs).not.toContain(" githits read "); + expect(docs.split("\n\nRead:")[0]).not.toContain(" githits read "); expect(docs).not.toContain("--lines"); const mcpDocs = renderUnifiedSearchSuccess(completed([docsHit()])); - expect(mcpDocs).not.toContain(" read target="); + expect(mcpDocs.split("\n\nRead:")[0]).not.toContain(" read target="); expect(mcpDocs).not.toContain("start_line="); expect(mcpDocs).not.toContain("end_line="); @@ -1784,7 +1790,7 @@ describe("renderUnifiedSearchSuccess", () => { }), { actionSyntax: "cli" }, ); - expect(empty).toContain("use --source symbol"); + expect(empty.replace(/\s+/g, " ")).toContain("use --source symbol"); expect(empty).toContain("use githits grep"); expect(empty).not.toContain('source="symbol"'); expect(empty).not.toContain("code_grep"); @@ -1806,11 +1812,9 @@ describe("renderUnifiedSearchSuccess", () => { }), ); - expect(firstLine(text)).toBe( - "No result snapshot yet | indexing | 0/2 ready", - ); + expect(firstLine(text)).toBe("No results available yet."); expect(text).toContain( - 'Next: search_status search_ref="ref_abc-123" wait_timeout_ms=30000', + 'search_status search_ref="ref_abc-123" wait_timeout_ms=30000', ); }); @@ -1834,16 +1838,14 @@ describe("renderUnifiedSearchSuccess", () => { }), ); - expect(firstLine(text)).toBe( - "No result snapshot yet | indexing | 0/1 ready", - ); + expect(firstLine(text)).toBe("No results available yet."); expect(text).not.toContain("Waiting:"); expect(text).not.toContain("Searched:"); expect(text).not.toContain("n8n.io"); expect(text).toContain("indexed: versions 2.26.9"); expect(text).toContain("versions 2.26.9"); expect(text).toContain( - 'Next: search_status search_ref="ref_abc-123" wait_timeout_ms=30000', + 'search_status search_ref="ref_abc-123" wait_timeout_ms=30000', ); }); @@ -1919,7 +1921,7 @@ describe("renderUnifiedSearchSuccess", () => { expect(text).toContain("- npm:express"); expect(text.match(/^- npm:express/gm)).toHaveLength(2); expect(text).toContain( - 'Next: search_status search_ref="ref_abc-123" wait_timeout_ms=30000', + 'search_status search_ref="ref_abc-123" wait_timeout_ms=30000', ); }); @@ -1930,9 +1932,7 @@ describe("renderUnifiedSearchSuccess", () => { }), ); - expect(firstLine(text)).toBe( - "No result snapshot yet | indexing | 0/1 ready", - ); + expect(firstLine(text)).toBe("No results available yet."); expect(text).toContain("Warnings:\n - unknown qualifier"); expect(text.match(/unknown qualifier/g)).toHaveLength(1); expect(text.indexOf("Warnings:")).toBeGreaterThan(0); @@ -1988,7 +1988,7 @@ describe("renderUnifiedSearchSuccess", () => { "available: site:docs.example.com, site:api.example.com", ); expect(text).toContain( - 'Next: search_status search_ref="ref_abc-123" wait_timeout_ms=30000', + 'search_status search_ref="ref_abc-123" wait_timeout_ms=30000', ); expect(text).not.toContain("Next: retry one suggested site target"); expect(text.match(/available:/g)).toHaveLength(1); @@ -2043,7 +2043,7 @@ describe("renderUnifiedSearchSuccess", () => { }), ); expect(terminalText).toContain("Try: site:docs.example.com"); - expect(terminalText).not.toContain("Next: search_status"); + expect(terminalText).not.toContain("search_status"); }); it("disambiguates multi-target readiness and preserves docs provenance", () => { @@ -2102,7 +2102,7 @@ describe("renderUnifiedSearchSuccess", () => { }), ); - expect(firstLine(text)).toBe("No results"); + expect(firstLine(text)).toBe("No results found."); expect(text).toContain("- npm:one@1.0.0\n indexing when observed: code"); expect(text).toContain("- npm:two@2.0.0\n indexing when observed: code"); }); @@ -2188,7 +2188,7 @@ describe("renderUnifiedSearchSuccess", () => { expect(text).toContain("not found: symbols"); expect(text).not.toContain("Fix:"); expect(text).toContain( - 'Next: shorten or broaden query; use source="symbol"; use grep.', + 'Try: shorten or broaden query; use source="symbol"; use grep.', ); }); @@ -2218,10 +2218,10 @@ describe("renderUnifiedSearchSuccess", () => { ); expect(text).toBe( - "No results | failed | 0/1 ready\n\nSources:\n - npm:express@4.18.2 (code, no results)\n\n" + + "No results found.\nSearch failed.\n\nSources:\n - npm:express@4.18.2 (code, no results)\n\n" + "- npm:express@4.18.2\n" + " not found: symbols\n\n" + - "Next: search again later.", + "Follow-up:\n Search again later.", ); }); @@ -2252,12 +2252,12 @@ describe("renderUnifiedSearchSuccess", () => { ); expect(text).toBe( - "No results\n\nSources:\n - npm:one@1.0.0 (code, no results)\n\n" + + "No results found.\n\nSources:\n - npm:one@1.0.0 (code, no results)\n\n" + "- npm:one@1.0.0\n" + " unresolved: symbols\n\n" + "- npm:two@2.0.0\n" + " indexing when observed: code\n\n" + - "Next: search again later.", + "Follow-up:\n Search again later.", ); }); @@ -2291,10 +2291,10 @@ describe("renderUnifiedSearchSuccess", () => { ); expect(text).toBe( - "No results | failed | 0/1 ready\n\nSources:\n - npm:express@4.18.2 (code, no results)\n\n" + + "No results found.\nSearch failed.\n\nSources:\n - npm:express@4.18.2 (code, no results)\n\n" + "- npm:express@4.18.2\n" + " unresolved: symbols; indexed: versions 4.17.0\n\n" + - "Next: search again later.", + "Follow-up:\n Search again later.", ); }); @@ -2306,7 +2306,7 @@ describe("renderUnifiedSearchSuccess", () => { ]), ); - expect(firstLine(text)).toBe("2 results | 2 repo code hits"); + expect(firstLine(text)).toBe("Found 2 code results."); expect(firstLine(text)).not.toContain(" from "); }); @@ -2327,17 +2327,15 @@ describe("renderUnifiedSearchSuccess", () => { }, }), ); - expect(firstLine(text)).toBe( - `No result snapshot yet | ${label} | 0/1 ready`, - ); + expect(firstLine(text)).toBe("No results available yet."); }, ); it.each([ - [false, "1 result"], - [true, "1 partial result"], + [false, "Found 1"], + [true, "Found 1 code result."], ] as const)( - "labels visible results independently of active indexing: %s", + "keeps outcome counts independent of backend partial truth: %s", (partial, label) => { const text = renderUnifiedSearchSuccess( incomplete({ @@ -2369,12 +2367,10 @@ describe("renderUnifiedSearchSuccess", () => { }, }), ); - expect(firstLine(text)).toBe( - `No result snapshot | ${status.toLowerCase()} | 0/1 ready`, - ); - expect(text).toContain("Next: search again later."); + expect(firstLine(text)).toBe("No result snapshot available."); + expect(text).toContain("Search again later."); expect(text).not.toContain("Do not poll"); - expect(text).not.toContain("Next: search_status"); + expect(text).not.toContain("search_status"); }, ); @@ -2389,12 +2385,10 @@ describe("renderUnifiedSearchSuccess", () => { }, }), ); - expect(firstLine(text)).toBe( - "No result snapshot | status unknown | 1/2 ready", - ); - expect(text).toContain("Next: search again later."); + expect(firstLine(text)).toBe("No result snapshot available."); + expect(text).toContain("Search again later."); expect(text).not.toContain("Do not poll"); - expect(text).not.toContain("Next: search_status"); + expect(text).not.toContain("search_status"); expect(text).not.toContain("FUTURE_SESSION_STATE"); }); @@ -2457,7 +2451,7 @@ describe("renderUnifiedSearchSuccess", () => { ), ); - expect(firstLine(text)).toBe("1 result | 1 repo code hit"); + expect(firstLine(text)).toBe("Found 1 code result."); expect(text).toContain("requested: npm:express latest"); expect(text.match(/older snapshot/g)).toHaveLength(1); expect(text).toContain( @@ -2478,7 +2472,7 @@ describe("renderUnifiedSearchSuccess", () => { ]), ); - expect(firstLine(text)).toBe("1 result | 1 repo code hit"); + expect(firstLine(text)).toBe("Found 1 code result."); expect(text).toContain("- npm:express latest"); expect(text.match(/using:/g)).toHaveLength(1); expect(text).toContain("using: 5.1.0 while 5.2.1 indexes"); @@ -2556,7 +2550,7 @@ describe("renderUnifiedSearchSuccess", () => { }), ); - expect(firstLine(text)).toBe("No results"); + expect(firstLine(text)).toBe("No results found."); }); it.each([ @@ -2589,7 +2583,7 @@ describe("renderUnifiedSearchSuccess", () => { }), ); - expect(firstLine(text)).toBe("No results"); + expect(firstLine(text)).toBe("No results found."); expect(firstLine(text)).not.toContain(targetLabel); expect(firstLine(text)).not.toContain(freshTarget); }, @@ -2602,8 +2596,8 @@ describe("renderUnifiedSearchSuccess", () => { evidenceNotice: "Opaque backend prose must not be copied.", }), ); - expect(firstLine(text)).toBe("No results"); - expect(text).toContain("Next: search again later."); + expect(firstLine(text)).toBe("No results found."); + expect(text).toContain("Search again later."); expect(text).not.toContain("Opaque backend prose"); expect(text).not.toContain("Evidence may change."); expect(text).not.toContain("Do not repeat"); @@ -2616,10 +2610,12 @@ describe("renderUnifiedSearchSuccess", () => { evidenceNotice: "Opaque backend prose must not be copied.", }), ); - expect(firstLine(text)).toContain("1 result"); + expect(firstLine(text)).toContain("Found 1"); expect(text).toContain("For updated results, search again."); const lines = text.split("\n"); - const actionLine = lines.findIndex((line) => line.startsWith("Next: ")); + const actionLine = lines.findIndex( + (line) => line === "Read:" || line === "Follow-up:", + ); expect(actionLine).toBeGreaterThan(0); expect(lines[actionLine - 1]).toBe(""); expect(text).toContain("applyEdit"); @@ -2644,7 +2640,7 @@ describe("renderUnifiedSearchSuccess", () => { ], }), ); - expect(firstLine(text)).toBe("No results"); + expect(firstLine(text)).toBe("No results found."); expect(text).toContain("Warnings:"); expect(text).toContain("kind was ignored by the selected source"); expect(text).toContain("incompatible query feature (code): kind"); @@ -2750,10 +2746,10 @@ describe("renderUnifiedSearchSuccess", () => { "[2] https://aider.chat/docs/more/edit-formats.html [docs page] aider-AI/aider -\n Edit Formats", ); expect(text).toContain( - "indexed: versions 5.2.1, 5.2.0, 5.1.0 +1, refs HEAD, main,", + "indexed: versions 5.2.1, 5.2.0, 5.1.0 (+1 more), refs HEAD, main,", ); - expect(text).toContain("next_offset=10"); - expect(cliText).toContain("next_offset=10"); + expect(text).toContain("offset=10"); + expect(cliText).toContain("--offset 10"); expect(cliText).not.toContain("More hits available"); expect(text).not.toContain("v5.0.0"); expect(text).not.toContain("dev"); @@ -2768,10 +2764,10 @@ describe("renderUnifiedSearchSuccess", () => { }); expect(presentation.hasMore).toBe(true); - expect(text).toContain("No results | next_offset=10"); + expect(text).toContain("No results on this page."); }); - it("keeps pagination in active and terminal result headlines", () => { + it("keeps pagination independent of active and terminal outcomes", () => { const active = renderUnifiedSearchSuccess( incomplete({ partialResults: false, @@ -2796,8 +2792,11 @@ describe("renderUnifiedSearchSuccess", () => { }), ); - expect(firstLine(active)).toContain("next_offset=10"); - expect(firstLine(terminal)).toContain("next_offset=10"); + expect(active).toContain("\nMore results:\n"); + expect(active).toContain("offset=10"); + expect(firstLine(active)).not.toContain("offset"); + expect(terminal).toContain("offset=10"); + expect(firstLine(terminal)).not.toContain("offset"); }); it("wraps bounded summaries without splitting exact tokens", () => { @@ -2862,7 +2861,11 @@ describe("renderUnifiedSearchSuccess", () => { expect(detailLines(narrow).length).toBeGreaterThan( detailLines(wide).length, ); - expect(detailLines(narrow).every((line) => line.length <= 60)).toBe(true); + expect( + detailLines(narrow) + .filter((line) => !line.trimStart().startsWith("search_status ")) + .every((line) => line.length <= 60), + ).toBe(true); expect(detailLines(wide).every((line) => line.length <= 140)).toBe(true); expect(wide).toContain( "n8n.io docs (1,480 pages; capped); indexed: versions", @@ -2912,7 +2915,10 @@ describe("renderUnifiedSearchSuccess", () => { ]), ); for (const line of text.split("\n")) { - if (!line.startsWith("[")) { + if ( + !line.startsWith("[") && + !line.trimStart().startsWith("read target=") + ) { expect(line.length).toBeLessThanOrEqual(82); } } @@ -3027,9 +3033,7 @@ describe("search preparation sections", () => { }), { width: 60 }, ); - expect(text.split("\n")[0]).toContain( - status.toLowerCase() === "pending" ? "preparing" : status.toLowerCase(), - ); + expect(text.split("\n")[0]).toMatch(/^Found 1 .* result\.$/); expect(text).toContain("\n\nPreparing:\n - "); const section = text.split("Preparing:\n")[1]!.split("\n\n")[0]!; const lines = section.split("\n"); @@ -3038,9 +3042,69 @@ describe("search preparation sections", () => { expect(lines.every((line) => line.length <= 60)).toBe(true); expect(section).toContain("indexing"); expect(section).toContain("preparing documentation"); - expect(text).toContain("1 result"); + expect(text).toContain("Found 1"); expect(text).toContain("search_status"); expect(text).not.toContain("remaining ETA"); }, ); }); + +describe("concise search outcome and footer contract", () => { + it("counts result kinds once and orders native read, pagination and wait actions", () => { + const payload = incomplete({ + partialResults: true, + results: [codeHit(), codeHit(), docsHit()], + hasMore: true, + nextOffset: 17, + }); + const before = structuredClone(payload); + for (const actionSyntax of ["mcp", "cli"] as const) { + const text = renderUnifiedSearchSuccess(payload, { actionSyntax }); + expect(firstLine(text)).toBe( + "Found 2 code results and 1 documentation result.", + ); + expect(text.indexOf("\nRead:")).toBeLessThan( + text.indexOf("\nMore results:"), + ); + expect(text.indexOf("\nMore results:")).toBeLessThan( + text.indexOf("\nFollow-up:"), + ); + expect(text).toContain( + actionSyntax === "mcp" ? "offset=17" : "--offset 17", + ); + expect(text).toContain("Use these results now; example read:"); + expect(text).toContain("If you need updated results, wait"); + expect(firstLine(text)).not.toMatch(/\||partial|ready|offset/); + } + expect(payload).toEqual(before); + }); + it.each([false, true])( + "explains completed partial snapshots without guessing a cause: %s", + (hasHit) => { + const results = hasHit ? [docsHit()] : []; + const text = renderUnifiedSearchSuccess( + completed(results, { partialResults: true }), + ); + expect(text).toContain("These results do not cover the full request."); + expect(text).not.toContain("indexing"); + expect(text).not.toContain("search_status"); + }, + ); + it("does not invent an offset when the backend omitted it", () => { + const text = renderUnifiedSearchSuccess( + completed([docsHit()], { hasMore: true, nextOffset: undefined }), + ); + expect(text).toContain( + "More results are available; repeat the original search.", + ); + expect(text).not.toContain("offset="); + expect(text).not.toContain("search_status"); + }); + it("keeps healthy results brief and offers a concrete read", () => { + const text = renderUnifiedSearchSuccess(completed([docsHit()])); + expect(firstLine(text)).toBe("Found 1 documentation result."); + expect(text).toContain("\nRead:\n read target="); + expect(text).not.toContain("Use these results now"); + expect(text).not.toContain("Follow-up:"); + }); +}); diff --git a/packages/mcp/src/shared/unified-search-text.ts b/packages/mcp/src/shared/unified-search-text.ts index 3ecd45e3..ea0fa441 100644 --- a/packages/mcp/src/shared/unified-search-text.ts +++ b/packages/mcp/src/shared/unified-search-text.ts @@ -21,6 +21,11 @@ import type { MappedError } from "./mapped-error.js"; import { formatMappedErrorText } from "./mapped-error-text.js"; import { renderReadTarget } from "./read-target-text.js"; import { parseRepositoryTargetSpec } from "./repository-target.js"; +import { + appendSearchGrepFooter, + footerAction, + footerProse, +} from "./search-grep-output-text.js"; import { formatProvenanceRow, formatRequestedIndexingExplanation, @@ -32,7 +37,6 @@ import { projectUnifiedSearchPresentation, targetDisplayFamilyKey, type UnifiedSearchAction, - type UnifiedSearchLifecycle, type UnifiedSearchPresentation, type UnifiedSearchSourceEntry, type UnifiedSearchSourceGroup, @@ -51,7 +55,6 @@ import type { } from "./unified-search-response.js"; const DEFAULT_TEXT_WIDTH = 80; -const SEP = " | "; type SearchSuccessPayload = | UnifiedSearchCompletedPresentation @@ -90,14 +93,12 @@ export function renderUnifiedSearchPresentationText( options: UnifiedSearchTextOptions = {}, ): string { const settings = normalizeTextOptions(options); - const lines: string[] = [ - formatPresentationOutcome( - presentation, - result.results, - result.nextOffset, - settings, - ), - ]; + const lines = wrapTerminalProse( + formatPresentationOutcome(presentation, result.results), + settings.width, + ).map((line) => styleOutcome(line, presentation, settings.useColors)); + const notice = formatPresentationNotice(presentation); + if (notice) lines.push(...wrapTerminalProse(notice, settings.width)); appendPresentationContext(lines, presentation, settings); if (result.results.length > 0) { @@ -110,7 +111,7 @@ export function renderUnifiedSearchPresentationText( ); } - appendPresentationAction(lines, presentation, result.results, settings); + appendPresentationAction(lines, presentation, result, settings); return lines.join("\n"); } @@ -136,129 +137,81 @@ function normalizeTextOptions( function formatPresentationOutcome( presentation: UnifiedSearchPresentation, results: UnifiedSearchHitPresentation[], - nextOffset: number | undefined, - options: NormalizedTextOptions, ): string { - const count = presentation.availability.resultCount; - const countLabel = `${count} result${count === 1 ? "" : "s"}`; - const finish = (value: string): string => - styleOutcome( - appendPagination(value, presentation.hasMore, nextOffset), - presentation, - options.useColors, - ); - - if (presentation.lifecycle.kind === "active") { - const label = activeLifecycleLabel(presentation.lifecycle); - const readiness = presentation.progress - ? `${presentation.progress.targetsReady}/${presentation.progress.targetsTotal} ready` - : undefined; - if (presentation.availability.kind === "no_snapshot") { - return finish( - ["No result snapshot yet", label, readiness].filter(Boolean).join(SEP), - ); - } - if (presentation.availability.kind === "empty") { - return finish( - ["No results yet", label, readiness].filter(Boolean).join(SEP), - ); - } - const resultLabel = - presentation.availability.kind === "partial" - ? countLabel.replace("result", "partial result") - : countLabel; - return finish( - [resultLabel, formatResultBreakdown(results), label, readiness] - .filter(Boolean) - .join(SEP), - ); - } - - if (presentation.lifecycle.kind === "completed") { - return finish( - count > 0 - ? formatCompletedResultsHeadline(results, countLabel) - : "No results", - ); - } - - const status = formatLifecycleSummary(presentation.lifecycle); - const readiness = presentation.progress - ? `${presentation.progress.targetsReady}/${presentation.progress.targetsTotal} ready` - : undefined; - if (count > 0) { - return finish( - [countLabel, formatResultBreakdown(results), status, readiness] - .filter(Boolean) - .join(SEP), - ); + if (!results.length) { + if (presentation.hasMore) return "No results on this page."; + if (presentation.lifecycle.kind === "active") + return "No results available yet."; + return presentation.availability.hasSnapshot + ? "No results found." + : "No result snapshot available."; } - if (presentation.availability.kind === "no_snapshot") { - return finish( - ["No result snapshot", status, readiness].filter(Boolean).join(SEP), - ); - } - return finish(["No results", status, readiness].filter(Boolean).join(SEP)); -} - -function formatCompletedResultsHeadline( - results: UnifiedSearchHitPresentation[], - countLabel: string, -): string { - const parts = [countLabel]; - const breakdown = formatResultBreakdown(results); - if (breakdown) parts.push(breakdown); - return parts.join(SEP); -} - -function appendPagination( - value: string, - hasMore: boolean, - nextOffset: number | undefined, -): string { - if (!hasMore) return value; - const field = - typeof nextOffset === "number" - ? `next_offset=${nextOffset}` - : "more available"; - return `${value}${SEP}${field}`; -} - -function formatResultBreakdown( - results: UnifiedSearchHitPresentation[], -): string { const counts = new Map(); for (const result of results) { - const label = resultBreakdownLabel(result.type); + const label = + result.type === "repository_code" + ? "code result" + : result.type === "documentation_page" + ? "documentation result" + : result.type === "repository_doc" + ? "repository documentation result" + : result.type === "repository_symbol" + ? "symbol result" + : "result"; counts.set(label, (counts.get(label) ?? 0) + 1); } - return [...counts.entries()] - .map(([label, count]) => `${count} ${resultCountLabel(label, count)}`) - .join(", "); -} - -function resultCountLabel(label: string, count: number): string { - if (count !== 1) return label; - if (label === "repo docs") return "repo doc"; - if (label === "docs pages") return "docs page"; - if (label === "repo code hits") return "repo code hit"; - if (label === "repo symbols") return "repo symbol"; - return label; + const parts = [...counts].map( + ([label, count]) => `${count} ${label}${count === 1 ? "" : "s"}`, + ); + const last = parts.pop(); + return `Found ${parts.length ? `${parts.join(", ")} and ` : ""}${last}.`; } -function resultBreakdownLabel(type: string): string { - switch (type) { - case "repository_doc": - return "repo docs"; - case "documentation_page": - return "docs pages"; - case "repository_symbol": - return "repo symbols"; - case "repository_code": - return "repo code hits"; - default: - return type; - } +/** Emit a notice only when source/preparation sections cannot convey the fact. */ +function formatPresentationNotice( + presentation: UnifiedSearchPresentation, +): string | undefined { + const scopeExplanation = + Boolean(presentation.indexingEstimates?.length) || + presentation.targetGroups.some( + (group) => + group.recovery || + group.sources.some((source) => + source.entries.some((entry) => entry.state !== "searched"), + ) || + group.trustLimits.some( + (limit) => + ["source", "coverage", "stale", "provisional"].includes( + limit.kind, + ) || + (limit.kind === "repository_snapshot" && + (limit.requestedCommitDiffers || limit.indexingRequestedRef)), + ), + ); + const incomplete = + presentation.availability.partialResults && !scopeExplanation; + const lifecycle = presentation.lifecycle; + const state = + lifecycle.kind === "terminal" + ? lifecycle.status === "FAILED" + ? "Search failed" + : lifecycle.status === "TIMEOUT" + ? "Search timed out" + : "Search was deferred" + : lifecycle.kind === "unknown" + ? "Search status is unknown" + : lifecycle.kind === "active" && !scopeExplanation && !incomplete + ? lifecycle.status === "INDEXING" + ? "Requested sources are still indexing" + : lifecycle.status === "PENDING" + ? "Search is preparing requested sources" + : "Search is still running" + : undefined; + if (state) + return `${state}${incomplete ? "; these results do not cover the full request" : ""}.`; + return incomplete + ? "These results do not cover the full request." + : undefined; } function styleOutcome( @@ -283,19 +236,6 @@ function styleOutcome( return `${colors.bold}${value}${colors.reset}`; } -function activeLifecycleLabel( - lifecycle: Extract, -): string { - switch (lifecycle.status) { - case "PENDING": - return "preparing"; - case "INDEXING": - return "indexing"; - case "SEARCHING": - return "searching"; - } -} - function appendPresentationContext( lines: string[], presentation: UnifiedSearchPresentation, @@ -885,93 +825,88 @@ function appendPresentationWarnings( } } -function formatLifecycleSummary(lifecycle: UnifiedSearchLifecycle): string { - if (lifecycle.kind === "completed") return "completed"; - if (lifecycle.kind === "active") return lifecycle.status.toLowerCase(); - if (lifecycle.kind === "terminal") return lifecycle.status.toLowerCase(); - return "status unknown"; -} - function formatRemaining(count: number): string { - return count > 0 ? ` +${count}` : ""; + return count > 0 ? ` (+${count} more)` : ""; } function appendPresentationAction( lines: string[], presentation: UnifiedSearchPresentation, - results: UnifiedSearchHitPresentation[], + result: UnifiedSearchTextResult, options: NormalizedTextOptions, ): void { const action = presentation.action; - if (action.kind === "none") return; - if (lines[lines.length - 1] !== "") { - lines.push(""); - } const useResults = "useResults" in action && action.useResults; + const read: string[] = []; + const more: string[] = []; + const followUp: string[] = []; + const prose = (value: string): string[] => footerProse(value, options.width); + const operand = (value: string): string => + footerAction(value, options.useColors); + const hit = result.results.find((hit) => hit.readTarget); const priorHead = presentation.targetGroups - .flatMap((group) => - group.trustLimits.filter((limit) => limit.kind === "repository_snapshot"), - ) - .find((snapshot) => snapshot.priorHead); - if (useResults) { - const hit = results.find((hit) => hit.readTarget); - lines.push( - ...wrapText( - hit - ? "Next: use these hits now; read for details:" - : "Next: use these hits now.", - options.width, + .flatMap((group) => group.trustLimits) + .find((limit) => limit.kind === "repository_snapshot" && limit.priorHead); + if (hit?.readTarget) { + if (useResults) read.push(...prose("Use these results now; example read:")); + read.push(operand(renderReadTarget(hit.readTarget, options.actionSyntax))); + } else if (useResults) followUp.push(...prose("Use these results now.")); + if (useResults && priorHead?.kind === "repository_snapshot") { + (read.length ? read : followUp).push( + ...prose( + `For a specific ref, search ${priorHead.commitTarget.replace(/@[^@]+$/, "@")}.`, ), ); - if (hit?.readTarget) - lines.push(renderReadTarget(hit.readTarget, options.actionSyntax)); - if (priorHead) { - lines.push( - ...wrapText( - `For a specific ref, search ${priorHead.commitTarget.replace(/@[^@]+$/, "@")}.`, - options.width, + } + if (presentation.hasMore) { + if (typeof result.nextOffset === "number") { + more.push( + ...prose("Repeat the original search, adding:"), + operand( + options.actionSyntax === "cli" + ? `--offset ${result.nextOffset}` + : `offset=${result.nextOffset}`, ), ); - } + } else + more.push( + ...prose("More results are available; repeat the original search."), + ); + if (presentation.lifecycle.kind === "active") + more.push(...prose("Results may change while this search is running.")); } if (action.kind === "poll") { - const next = - options.actionSyntax === "cli" - ? `Next: githits search-status ${action.searchRef} --wait ${action.waitTimeoutMs / 1000}` - : `Next: search_status search_ref=${JSON.stringify(action.searchRef)} wait_timeout_ms=${action.waitTimeoutMs}`; - if (useResults) { - lines.push( - ...wrapText( + if (useResults) + followUp.push( + ...prose( priorHead ? "If you need current HEAD, wait (hits and order may change):" : "If you need updated results, wait (hits and order may change):", - options.width, ), ); - } - lines.push( - highlight( - useResults ? next.replace("Next: ", "") : next, - options.useColors, + followUp.push( + operand( + options.actionSyntax === "cli" + ? `githits search-status ${action.searchRef} --wait ${action.waitTimeoutMs / 1000}` + : `search_status search_ref=${JSON.stringify(action.searchRef)} wait_timeout_ms=${action.waitTimeoutMs}`, ), ); - return; - } - if (action.kind === "new_search") { - lines.push( - useResults - ? "For updated results, search again." - : "Next: search again later.", + } else if (action.kind === "new_search") { + followUp.push( + ...prose( + useResults + ? "For updated results, search again." + : "Search again later.", + ), ); - return; - } - if (action.kind === "query_rewrite") { - lines.push( - `Next: ${action.rewrites - .map((rewrite) => formatRewrite(rewrite, options.actionSyntax)) - .join("; ")}.`, + } else if (action.kind === "query_rewrite") { + followUp.push( + ...prose( + `Try: ${action.rewrites.map((rewrite) => formatRewrite(rewrite, options.actionSyntax)).join("; ")}.`, + ), ); } + appendSearchGrepFooter(lines, { read, more, followUp }, options.useColors); } function formatRewrite( diff --git a/packages/mcp/src/smoke-test.test.ts b/packages/mcp/src/smoke-test.test.ts index 5eb81082..bf592762 100644 --- a/packages/mcp/src/smoke-test.test.ts +++ b/packages/mcp/src/smoke-test.test.ts @@ -686,7 +686,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "1 result | 1 repo code hit\n\n[1] npm:express@4.21.2 examples/route-middleware/index.js [repo code, path match]\n 50 | function andRestrictTo(role) {", + "Found 1 code result.\n\n[1] npm:express@4.21.2 examples/route-middleware/index.js [repo code, path match]\n 50 | function andRestrictTo(role) {", ); } return smokeResponse(name, args); @@ -700,7 +700,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "1 result | 1 repo code hit\n\n[1] npm:express@4.21.2 examples/route-middleware/index.js [repo code, path match]", + "Found 1 code result.\n\n[1] npm:express@4.21.2 examples/route-middleware/index.js [repo code, path match]", ); } return smokeResponse(name, args); @@ -712,7 +712,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "2 results | 2 repo code hits\n\n[1] npm:express@4.21.2 examples/route-middleware/index.js [repo code, path match]\n\n[2] npm:express@4.21.2 lib/router/index.js:303-307 [repo code]\n> 305 | // route", + "Found 2 code results.\n\n[1] npm:express@4.21.2 examples/route-middleware/index.js [repo code, path match]\n\n[2] npm:express@4.21.2 lib/router/index.js:303-307 [repo code]\n> 305 | // route", ); } return smokeResponse(name, args); @@ -742,14 +742,14 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - 'No result snapshot yet | indexing | 0/1 ready\nNext: search_status search_ref="smoke-ref" wait_timeout_ms=30000\nsearch_ref=leaked', + 'No results available yet.\nFollow-up:\n search_status search_ref="smoke-ref" wait_timeout_ms=30000\nsearch_ref=leaked', ); } return smokeResponse(name, args); }); await expect(runMcpSmoke(caller)).rejects.toThrow( - "search default: search_ref= must appear at most once", + "search default: expected at most one native status action", ); }); @@ -774,7 +774,7 @@ describe("runMcpSmoke", () => { ); it.each([ - "Next: githits search-status smoke-ref --wait 30", + "Follow-up:\n githits search-status smoke-ref --wait 30", "Next: githits read npm:express index.js", "Next: githits code read npm:express index.js", "Next: githits docs read page-1 --offset 10", @@ -783,7 +783,7 @@ describe("runMcpSmoke", () => { if (name === "search" && args.format !== "json") { return textResult( smokeSearchText().replace( - 'Next: search_status search_ref="smoke-ref" wait_timeout_ms=30000', + 'Follow-up:\n search_status search_ref="smoke-ref" wait_timeout_ms=30000', action, ), ); @@ -876,10 +876,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - smokeSearchText().replace( - "No result snapshot yet | indexing | 0/1 ready", - "Warnings:", - ), + smokeSearchText().replace("No results available yet.", "Warnings:"), ); } return smokeResponse(name, args); @@ -894,7 +891,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "1 result\n\n[1] npm:express@5.2.1 index.js [repo code]\n" + + "Found 1 code result.\n\n[1] npm:express@5.2.1 index.js [repo code]\n" + " Ready: payload text\n" + " Waiting: payload text\n" + " Available but not searched: payload text\n" + @@ -920,7 +917,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "1 result | 1 repo code hit\n\n[1] npm:express@5.2.1 index.js [repo code]\n" + + "Found 1 code result.\n\n[1] npm:express@5.2.1 index.js [repo code]\n" + " First summary paragraph.\n\n" + " status: payload text\n" + " searchRef=payload text\n" + @@ -938,7 +935,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "1 result\n\n[1] npm:express@5.2.1 index.js [repo code]", + "Found 1 code result.\n\n[1] npm:express@5.2.1 index.js [repo code]", ); } return smokeResponse(name, args); @@ -951,7 +948,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "1 result\n\n[1] page-1 [docs page] npm:express - docs.example.com/readme - README | API - section", + "Found 1 code result.\n\n[1] page-1 [docs page] npm:express - docs.example.com/readme - README | API - section", ); } return smokeResponse(name, args); @@ -964,7 +961,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "1 result | 1 docs page\n\n[1] page-1 [docs page] npm:express - source URL unavailable - README", + "Found 1 documentation result.\n\n[1] page-1 [docs page] npm:express - source URL unavailable - README", ); } return smokeResponse(name, args); @@ -977,7 +974,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "2 results | 1 repo code hit, 1 docs page\n\n" + + "Found 1 code result and 1 documentation result.\n\n" + "[1] page-1 [docs page] npm:express - docs.example.com/readme -\n" + " A long documentation title\n\n" + "[2] npm:express@5.2.1 lib/application.js [repo code] -\n" + @@ -994,7 +991,7 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "1 result | 1 repo code hit\n\n" + + "Found 1 code result.\n\n" + "[1] npm:express@5.2.1 lib/application.js [repo code] -\n" + " A long repository title", ); @@ -1008,13 +1005,13 @@ describe("runMcpSmoke", () => { it.each([ [ "focused evidence", - "1 result | 1 repo code hit\n\n" + + "Found 1 code result.\n\n" + "[1] github:owner/repo@abc123 packages/pkg/src/compact.ts:920-930 [repo code] - compact (function at lines 858-964)\n" + " // Merge into single summary", ], [ "equal evidence", - "1 result | 1 repo symbol\n\n" + + "Found 1 symbol result.\n\n" + "[1] github:owner/repo@abc123 packages/pkg/src/compact.ts:858-964 [repo symbol] - compact (function)", ], ])("allows a unified repository hit with %s", async (_name, searchText) => { @@ -1030,7 +1027,7 @@ describe("runMcpSmoke", () => { it("accepts structural search evidence text", async () => { const structuralText = - "1 result | 1 repo code hit\n\n" + + "Found 1 code result.\n\n" + "[1] npm:express@4.18.2 lib/client.ts:142-145 [repo code]\n" + " - class Client | lines 20-220\n" + " - method Client.send | lines 120-165\n" + @@ -1052,42 +1049,44 @@ describe("runMcpSmoke", () => { it.each([ [ - "1 result\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n" + + "Found 1 code result.\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n" + " This payload mentions read but has no locator", ], [ - "1 result\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n" + + "Found 1 code result.\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n" + ' read target="npm:express@5.2.1"', ], - ["1 result\n\n[1] page-1 [docs page] npm:express - README"], + ["Found 1 code result.\n\n[1] page-1 [docs page] npm:express - README"], [ - "1 result\n\n[1] page-1 [docs page] npm:express -\n" + + "Found 1 code result.\n\n[1] page-1 [docs page] npm:express -\n" + " README without a source locator", ], [ - "1 result\n\n[1] page ID unavailable [docs page] npm:express - docs.example.com/readme -\n" + + "Found 1 code result.\n\n[1] page ID unavailable [docs page] npm:express - docs.example.com/readme -\n" + " Wrapped title without a page locator", ], [ - "1 result\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n" + + "Found 1 code result.\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n" + " ordinary title\n" + ' read target="npm:express@5.2.1" path="index.js"', ], [ - "1 result\n\n[1] npm:express@5.2.1 location unavailable [repo code] -\n" + + "Found 1 code result.\n\n[1] npm:express@5.2.1 location unavailable [repo code] -\n" + " Wrapped title without a locator", ], - ["1 result\n\n[1] npm:express@5.2.1 lib/application.js [repo code] -"], [ - "1 result\n\n[1] compact - function defined at packages/pkg/src/compact.ts:858-964", + "Found 1 code result.\n\n[1] npm:express@5.2.1 lib/application.js [repo code] -", + ], + [ + "Found 1 code result.\n\n[1] compact - function defined at packages/pkg/src/compact.ts:858-964", ], [ - "1 result\n\n" + + "Found 1 code result.\n\n" + "[1] compact - function defined at packages/pkg/src/compact.ts:858-964\n" + " github:owner/repo@abc123 evidence at 920-930 [repo code]", ], [ - "1 result\n\n[1] compact - function defined at location unavailable\n" + + "Found 1 code result.\n\n[1] compact - function defined at location unavailable\n" + " github:owner/repo@abc123 evidence at 920-930 [repo code]", ], ])("rejects incomplete or prose-only hit follow-ups", async (searchText) => { @@ -1106,9 +1105,12 @@ describe("runMcpSmoke", () => { it.each([ [ "Fix", - "No results\n\n- npm:missing@1.0.0\n Fix: verify the package coordinate.", + "No results found.\n\n- npm:missing@1.0.0\n Fix: verify the package coordinate.", + ], + [ + "Try", + "No results found.\n\n- npm:missing latest\n Try: npm:missing@1.0.0", ], - ["Try", "No results\n\n- npm:missing latest\n Try: npm:missing@1.0.0"], ])( "accepts target-local %s recovery without a hit or Next", async (_kind, searchText) => { @@ -1127,10 +1129,10 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - "No results | failed | 0/1 ready\n\n" + + "No results found.\nSearch failed.\n\n" + "- npm:express@4.18.2\n" + " searched: code; not found: symbols\n\n" + - "Next: rerun search later.", + "Follow-up:\n Search again later.", ); } return smokeResponse(name, args); @@ -1149,7 +1151,8 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - `No results\n\n- npm:express@4.18.2\n ${detail}\n\nNext: rerun search later.`, + `No results found.\n\n- npm:express@4.18.2\n ${detail}\n\nFollow-up: + Search again later.`, ); } return smokeResponse(name, args); @@ -1168,7 +1171,8 @@ describe("runMcpSmoke", () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { return textResult( - `No results\n ${detail}\n\nNext: rerun search later.`, + `No results found.\n ${detail}\n\nFollow-up: + Search again later.`, ); } return smokeResponse(name, args); @@ -1179,18 +1183,16 @@ describe("runMcpSmoke", () => { ); }); - it("rejects duplicate lifecycle outcome lines", async () => { + it("rejects duplicate outcome lines", async () => { const caller = createCaller(async (name, args) => { if (name === "search" && args.format !== "json") { - return textResult( - `${smokeSearchText()}\nNo result snapshot yet | indexing | 0/1 ready`, - ); + return textResult(`${smokeSearchText()}\nNo results available yet.`); } return smokeResponse(name, args); }); await expect(runMcpSmoke(caller)).rejects.toThrow( - "search default: duplicate lifecycle outcome lines", + "search default: duplicate outcome lines", ); }); }); @@ -1417,10 +1419,10 @@ function smokeResponse( ); case "search": return textResult( - "No result snapshot yet | indexing | 0/1 ready\n\n" + + "No results available yet.\n\n" + "- npm:express@5.2.1\n" + " indexing: code; available: versions 5.2.1\n\n" + - 'Next: search_status search_ref="smoke-ref" wait_timeout_ms=30000', + 'Follow-up:\n search_status search_ref="smoke-ref" wait_timeout_ms=30000', ); case "search_status": return errorResult("NOT_FOUND"); @@ -1671,3 +1673,18 @@ function smokeJsonResponse( throw new Error(`unexpected smoke JSON tool ${name}`); } } + +describe("search MCP footer structure", () => { + it("rejects status operands without the Follow-up section", async () => { + const caller = createCaller(async (name, args) => + name === "search" && args.format !== "json" + ? textResult( + 'No results available yet.\n search_status search_ref="ref" wait_timeout_ms=30000', + ) + : smokeResponse(name, args), + ); + await expect(runMcpSmoke(caller)).rejects.toThrow( + "status action must be in Follow-up", + ); + }); +}); diff --git a/packages/mcp/src/smoke-test.ts b/packages/mcp/src/smoke-test.ts index 0bbe9e5b..5dc7e528 100644 --- a/packages/mcp/src/smoke-test.ts +++ b/packages/mcp/src/smoke-test.ts @@ -422,9 +422,7 @@ function assertSearchDefaultText(text: string, context: string): void { const firstLine = lines[0]?.trim() ?? ""; assert(firstLine.length > 0, `${context}: missing outcome first line`); assert( - /^(?:No result snapshot yet|No results yet|No result snapshot|No results)\b|^\d+ (?:partial |interim )?results?\b/.test( - firstLine, - ), + /^(?:Found \d+ |No results? )/.test(firstLine), `${context}: missing outcome headline`, ); assert( @@ -440,13 +438,10 @@ function assertSearchDefaultText(text: string, context: string): void { !formatterLines.some((line) => /^Search\s+\S+\s+\|/.test(line)), `${context}: separate Search session summary`, ); - const lifecycleOutcomeLines = lines.filter((line) => - /\|\s+(?:preparing|indexing|searching)(?:\s*\||$)/.test(line), - ); - assert( - lifecycleOutcomeLines.length <= 1, - `${context}: duplicate lifecycle outcome lines`, + const outcomeLines = formatterLines.filter((line) => + /^(?:Found \d+ |No results? )/.test(line), ); + assert(outcomeLines.length === 1, `${context}: duplicate outcome lines`); assert( !formatterText.includes("searchRef="), `${context}: leaked searchRef=`, @@ -481,6 +476,23 @@ function assertSearchDefaultText(text: string, context: string): void { `${context}: poll policy prose`, ); + const footerLabels = ["Read:", "More results:", "Follow-up:"]; + const presentFooters = formatterLines.filter((line) => + footerLabels.includes(line), + ); + assert( + new Set(presentFooters).size === presentFooters.length, + `${context}: duplicated footer section`, + ); + assert( + presentFooters.join() === + footerLabels.filter((label) => presentFooters.includes(label)).join(), + `${context}: footer sections out of order`, + ); + assert( + !firstLine.includes("|") && !/partial|interim|ready|offset/.test(firstLine), + `${context}: headline must contain only the outcome`, + ); const hasReadinessText = formatterLines.some((line) => TARGET_DETAIL_STATE_PATTERN.test(line), ); @@ -491,34 +503,28 @@ function assertSearchDefaultText(text: string, context: string): void { ); } - const nextLines = lines.filter((line) => line.startsWith("Next:")); - assert( - nextLines.length <= 1, - `${context}: multiple Next actions are not allowed`, + const statusActions = formatterLines.filter((line) => + line.startsWith(" search_status "), ); - const searchRefOccurrences = formatterText.match(/search_ref=/g)?.length ?? 0; assert( - searchRefOccurrences <= 1, - `${context}: search_ref= must appear at most once`, + statusActions.length <= 1 && searchRefOccurrences === statusActions.length, + `${context}: expected at most one native status action`, ); - if (searchRefOccurrences === 1) { - const refLine = formatterLines.find((line) => line.includes("search_ref=")); + const statusAction = statusActions[0]; + if (statusAction) { assert( - refLine?.trimStart().startsWith("Next:"), - `${context}: search_ref= must appear only on a Next line`, + formatterLines.includes("Follow-up:") && + formatterLines.indexOf("Follow-up:") < + formatterLines.indexOf(statusAction), + `${context}: status action must be in Follow-up`, ); assert( - refLine?.startsWith("Next: search_status "), - `${context}: search_ref= must use the MCP search_status action`, - ); - assert( - refLine !== undefined, - `${context}: search_ref= must appear only on a Next line`, + /^ {2}search_status search_ref="[^"]+" wait_timeout_ms=\d+$/.test( + statusAction, + ), + `${context}: invalid native status action`, ); - const match = refLine.match(/search_ref=(?:"([^"]+)"|(\S+))/); - const searchRef = match?.[1] ?? match?.[2]; - assert(searchRef !== undefined, `${context}: missing search_ref value`); } assert( !formatterText.includes("githits search-status ") && @@ -532,7 +538,8 @@ function assertSearchDefaultText(text: string, context: string): void { assert( hasHumanSearchHitLocator(lines) || hasTargetRecovery(formatterLines) || - lines.some((line) => line.startsWith("Next:")), + formatterLines.includes("Follow-up:") || + formatterLines.includes("More results:"), `${context}: missing usable result locator or status follow-up`, ); } @@ -1691,9 +1698,9 @@ async function runLiveSmoke(caller: McpSmokeCaller): Promise { } assert( grepText.includes("Sources:") && - grepText.includes("# Read files: read target=$target path=$path") && - grepText.includes("# Read pages: read target=$url") && - grepText.includes("More matches") && + grepText.includes("Files: read target=$target path=$path") && + grepText.includes("Pages: read target=$url") && + grepText.includes("More results:") && grepText.includes(`cursor=${JSON.stringify(grepJson.nextCursor)}`), "grep default missing mixed-source, exact-read, or continuation guidance", ); diff --git a/packages/mcp/src/tools/grep.test.ts b/packages/mcp/src/tools/grep.test.ts index e8ef3adb..a7f3f26a 100644 --- a/packages/mcp/src/tools/grep.test.ts +++ b/packages/mcp/src/tools/grep.test.ts @@ -139,7 +139,7 @@ describe("unified MCP grep", () => { }); expect(result.isError).toBeUndefined(); - expect(result.content[0]?.text).toBe("No matches."); + expect(result.content[0]?.text).toBe("No matches found."); expect(grep).toHaveBeenCalledTimes(1); expect(grep).toHaveBeenCalledWith({ targets: [ @@ -217,8 +217,8 @@ describe("unified MCP grep", () => { }); const text = result.content[0]?.text ?? ""; - expect(text).toContain("# Read files: read target=$target path=$path"); - expect(text).toContain("# Read pages: read target=$url"); + expect(text).toContain("Files: read target=$target path=$path"); + expect(text).toContain("Pages: read target=$url"); expect(text).toContain("Sources:"); expect(text).toContain('cursor="'); expect(text).not.toContain("githits read"); diff --git a/packages/mcp/src/tools/search-status.test.ts b/packages/mcp/src/tools/search-status.test.ts index 3e17b054..b2a01303 100644 --- a/packages/mcp/src/tools/search-status.test.ts +++ b/packages/mcp/src/tools/search-status.test.ts @@ -386,9 +386,9 @@ describe("searchStatusTool", () => { const result = await tool.handler({ search_ref: "ref-timeout" }, {}); const text = result.content[0]?.text ?? ""; - expect(text).toContain("No result snapshot | timeout | 0/1 ready"); + expect(text).toContain("No result snapshot available."); expect(text).not.toContain("search_status |"); - expect(text).toContain("Next: search again later."); + expect(text).toContain("Search again later."); expect(text).not.toContain("search_ref="); }); @@ -403,8 +403,8 @@ describe("searchStatusTool", () => { const result = await tool.handler({ search_ref: "ref-failed" }, {}); const text = result.content[0]?.text ?? ""; - expect(text).toContain("No result snapshot | failed | 0/1 ready"); - expect(text).toContain("Next: search again later."); + expect(text).toContain("No result snapshot available."); + expect(text).toContain("Search again later."); expect(text).not.toContain("search_ref="); }); @@ -446,7 +446,7 @@ describe("searchStatusTool", () => { const textResult = await tool.handler({ search_ref: "ref-deferred" }, {}); const text = textResult.content[0]?.text ?? ""; - expect(text).toContain("1 result | 1 repo code hit | deferred | 1/2 ready"); + expect(text).toContain("Found 1 code result."); expect(text).toContain("For updated results, search again."); expect(text).not.toContain("search_ref="); expect(text).not.toContain("No hits"); @@ -466,8 +466,8 @@ describe("searchStatusTool", () => { const result = await tool.handler({ search_ref: "ref-deferred-empty" }, {}); const text = result.content[0]?.text ?? ""; - expect(text).toContain("No result snapshot | deferred | 0/1 ready"); - expect(text).toContain("Next: search again later."); + expect(text).toContain("No result snapshot available."); + expect(text).toContain("Search again later."); expect(text).not.toContain("No hits"); expect(text).not.toContain("Indexing in progress"); expect(text).not.toContain("search_ref="); @@ -505,9 +505,7 @@ describe("searchStatusTool", () => { const textResult = await tool.handler({ search_ref: "ref-future" }, {}); const text = textResult.content[0]?.text ?? ""; - expect(text).toContain( - "1 result | 1 repo code hit | status unknown | 0/1 ready", - ); + expect(text).toContain("Found 1 code result."); expect(text).toContain("For updated results, search again."); expect(text).not.toContain("search_ref="); expect(text).not.toContain("No hits"); @@ -516,8 +514,14 @@ describe("searchStatusTool", () => { }); it.each([ - ["FAILED", "No results | failed | 0/1 ready"], - ["TIMEOUT", "No results | timeout | 0/1 ready"], + [ + "FAILED", + "No results found.\nSearch failed; these results do not cover the full request.", + ], + [ + "TIMEOUT", + "No results found.\nSearch timed out; these results do not cover the full request.", + ], ] as const)( "does not promise future hits for a terminal %s partial result", async (status, expectedMessage) => { @@ -587,7 +591,7 @@ describe("searchStatusTool", () => { const result = await tool.handler({ search_ref: incomplete.searchRef }, {}); const text = result.content[0]?.text ?? ""; - expect(text).toContain("No results yet | indexing | 0/1 ready"); + expect(text).toContain("No results available yet."); expect(text).toContain("- site:example.com"); expect(text).toContain( "Sources:\n - site:example.com (hosted documentation)", @@ -595,7 +599,7 @@ describe("searchStatusTool", () => { expect(text).toContain("available: site:docs.example.com"); expect(text).toContain("+more"); expect(text).toContain( - 'Next: search_status search_ref="ref-site-recovery" wait_timeout_ms=30000', + 'Follow-up:\n search_status search_ref="ref-site-recovery" wait_timeout_ms=30000', ); expect(text).not.toContain("Next: retry one suggested site target"); }); @@ -740,14 +744,16 @@ describe("searchStatusTool", () => { const result = await tool.handler({ search_ref: "search-ref-123" }, {}); const text = result.content[0]?.text ?? ""; - expect(text).toContain("No results"); + expect(text).toContain("No results found."); expect(text).toContain("- site:example.com"); expect(text).toContain( "Sources:\n - site:example.com (hosted documentation)", ); expect(text).toContain("Try: site:example.com/docs"); expect(text).toContain("+more"); - expect(text).not.toContain("Next: shorten or broaden site query."); + expect(text).not.toContain( + "Follow-up:\n Try: shorten or broaden site query.", + ); }); it("renders terminal source status compactly in completed text", async () => { @@ -797,7 +803,7 @@ describe("searchStatusTool", () => { {}, ); const text = result.content[0]?.text ?? ""; - expect(text).toContain("No results"); + expect(text).toContain("No results found."); expect(text).toContain("- github:githits-com/no-such-repo"); expect(text).toContain("repository unresolved: code"); expect(text).not.toContain("Searched: code"); @@ -818,10 +824,10 @@ describe("searchStatusTool", () => { const text = result.content[0]?.text ?? ""; expect(result.isError).toBeUndefined(); - expect(text).toContain("No result snapshot yet | searching | 0/1 ready"); + expect(text).toContain("No results available yet."); expect(text).not.toContain("Search ref-text |"); expect(text).toContain( - 'Next: search_status search_ref="ref-text" wait_timeout_ms=30000', + 'Follow-up:\n search_status search_ref="ref-text" wait_timeout_ms=30000', ); expect(text).not.toContain("search_status |"); expect(text).not.toContain("searchRef="); @@ -853,7 +859,7 @@ describe("searchStatusTool", () => { expect(text).toContain("- npm:express latest"); expect(text).toContain("indexed: versions 4.18.2, refs main"); expect(text).toContain( - 'Next: search_status search_ref="ref-alternatives" wait_timeout_ms=30000', + 'Follow-up:\n search_status search_ref="ref-alternatives" wait_timeout_ms=30000', ); expect(text).not.toContain("allow_partial_results: true"); }); diff --git a/packages/mcp/src/tools/search.test.ts b/packages/mcp/src/tools/search.test.ts index a2f9fa4d..0abc777a 100644 --- a/packages/mcp/src/tools/search.test.ts +++ b/packages/mcp/src/tools/search.test.ts @@ -749,7 +749,7 @@ describe("searchTool", () => { expect(result.isError).toBeUndefined(); const text = result.content[0]?.text ?? ""; expect(text.split("\n")[0]).not.toContain("search | "); - expect(text.split("\n")[0]).toContain("1 result"); + expect(text.split("\n")[0]).toContain("Found 1 code result."); // Confirm the text payload is not valid JSON. expect(() => JSON.parse(text)).toThrow(); }); diff --git a/scripts/cli-smoke.ts b/scripts/cli-smoke.ts index d5274aa4..89f9be9e 100644 --- a/scripts/cli-smoke.ts +++ b/scripts/cli-smoke.ts @@ -523,9 +523,7 @@ export function assertSearchTerminalText(text: string, context: string): void { `${context}: non-outcome text precedes search outcome`, ); assert( - /^(?:No result snapshot yet|No results yet|No result snapshot|No results)\b|^\d+ (?:partial |interim )?results?\b/.test( - firstLine, - ), + /^(?:Found \d+ |No results? )/.test(firstLine), `${context}: missing outcome headline`, ); assert( @@ -536,13 +534,10 @@ export function assertSearchTerminalText(text: string, context: string): void { !formatterLines.some((line) => /^Search\s+\S+\s+\|/.test(line)), `${context}: separate Search session summary`, ); - const lifecycleOutcomeLines = formatterLines.filter((line) => - /\|\s+(?:preparing|indexing|searching)(?:\s*\||$)/.test(line), - ); - assert( - lifecycleOutcomeLines.length <= 1, - `${context}: duplicate lifecycle outcome lines`, + const outcomeLines = formatterLines.filter((line) => + /^(?:Found \d+ |No results? )/.test(line), ); + assert(outcomeLines.length === 1, `${context}: duplicate outcome lines`); assert( !formatterText.includes("searchRef:") && !formatterText.includes("searchRef="), @@ -582,6 +577,23 @@ export function assertSearchTerminalText(text: string, context: string): void { `${context}: poll policy prose`, ); + const footerLabels = ["Read:", "More results:", "Follow-up:"]; + const presentFooters = formatterLines.filter((line) => + footerLabels.includes(line), + ); + assert( + new Set(presentFooters).size === presentFooters.length, + `${context}: duplicated footer section`, + ); + assert( + presentFooters.join() === + footerLabels.filter((label) => presentFooters.includes(label)).join(), + `${context}: footer sections out of order`, + ); + assert( + !firstLine.includes("|") && !/partial|interim|ready|offset/.test(firstLine), + `${context}: headline must contain only the outcome`, + ); const hasReadinessText = formatterLines.some((line) => TARGET_DETAIL_STATE_PATTERN.test(line), ); @@ -592,23 +604,25 @@ export function assertSearchTerminalText(text: string, context: string): void { ); } - const nextLines = formatterLines.filter((line) => line.startsWith("Next:")); - assert( - nextLines.length <= 1, - `${context}: multiple Next actions are not allowed`, - ); - const statusActions = nextLines.filter((line) => - line.startsWith("Next: githits search-status "), + const statusActions = formatterLines.filter((line) => + line.startsWith(" githits search-status "), ); assert( statusActions.length <= 1, `${context}: expected at most one search-status action`, ); - if (statusActions.length === 1) { - const searchRef = statusActions[0]?.match( - /^Next: githits search-status (\S+) /, - )?.[1]; - assert(searchRef !== undefined, `${context}: missing search-status ref`); + const statusAction = statusActions[0]; + if (statusAction) { + assert( + formatterLines.includes("Follow-up:") && + formatterLines.indexOf("Follow-up:") < + formatterLines.indexOf(statusAction), + `${context}: status action must be in Follow-up`, + ); + assert( + /^ {2}githits search-status \S+ --wait \d+(?:\.\d+)?$/.test(statusAction), + `${context}: invalid native status action`, + ); } assert( !formatterText.includes("search_ref="), @@ -617,7 +631,8 @@ export function assertSearchTerminalText(text: string, context: string): void { assert( hasHumanSearchHitLocator(lines) || hasTargetRecovery(formatterLines) || - nextLines.length > 0, + formatterLines.includes("Follow-up:") || + formatterLines.includes("More results:"), `${context}: missing result follow-up or next action`, ); } @@ -2111,7 +2126,7 @@ async function runLiveSmoke(env: Record): Promise { grepText.includes("lib/express.js") && /^\s*\d+: .*router/m.test(grepText) && grepText.includes( - "# Read files: read --lines $start-$end -- $target $path", + "Files: githits read --lines $start-$end -- $target $path", ) && !grepText.includes("# source [") && !grepText.includes("Read recipes") && diff --git a/scripts/smoke-scripts.test.ts b/scripts/smoke-scripts.test.ts index 4a3514ec..887438c1 100644 --- a/scripts/smoke-scripts.test.ts +++ b/scripts/smoke-scripts.test.ts @@ -30,42 +30,46 @@ import { import { toStdioLaunch } from "./smoke-launch-target.ts"; describe("CLI search smoke contract", () => { - const valid = `No results yet | indexing | 0/1 ready + const valid = `No results available yet. - npm:n8n indexing: code, repository docs; available: n8n.io docs (1,480 pages; capped); indexed: versions 2.26.9, 2.26.5, 2.23.2 +2, refs HEAD, master -Next: githits search-status smoke-ref --wait 20`; - const completedWithTargetReadiness = `No results +Follow-up: + githits search-status smoke-ref --wait 20`; + const completedWithTargetReadiness = `No results found. - npm:express@4.18.2 searched: repository docs -Next: shorten or broaden query; use githits grep.`; - const completed = `1 result | 1 repo code hit | next_offset=10 +Follow-up: + Try: shorten or broaden query; use githits grep.`; + const completed = `Found 1 code result. [1] npm:express@5.2.1 lib/application.js [repo code]`; - const completedDocs = `1 result | 1 docs page + const completedDocs = `Found 1 documentation result. [1] page-1 [docs page] npm:express - docs.example.com/getting-started - Getting started | API - section`; it("accepts outcome-first text with CLI-native actions", () => { - expect(valid.split("\n")[0]).toBe("No results yet | indexing | 0/1 ready"); + expect(valid.split("\n")[0]).toBe("No results available yet."); expect(valid).toContain("- npm:n8n"); expect(valid).toContain(" indexing: code, repository docs; available:"); expect(valid).not.toContain("Search smoke-ref"); - expect(valid).toContain("Next: githits search-status smoke-ref --wait 20"); + expect(valid).toContain( + "Follow-up:\n githits search-status smoke-ref --wait 20", + ); expect(() => assertSearchTerminalText(valid, "search")).not.toThrow(); expect(() => assertSearchTerminalText( - "No results\nNext: shorten or broaden query; use githits grep.", + "No results found.\nFollow-up:\n Try: shorten or broaden query; use githits grep.", "search", ), ).not.toThrow(); expect(() => assertSearchTerminalText( - "No result snapshot | failed | 0/1 ready\nNext: rerun search later.", + "No result snapshot available.\nFollow-up:\n Search again later.", "search", ), ).not.toThrow(); @@ -78,7 +82,7 @@ Next: shorten or broaden query; use githits grep.`; completed.replace(" lib/application.js [repo code]", ""), "missing result follow-up", ], - ["1 result from npm:express@5.2.1", "missing result follow-up"], + ["Found 1 code result.", "missing result follow-up"], ])("rejects invalid search text", (text, message) => { expect(() => assertSearchTerminalText(text, "search")).toThrow(message); }); @@ -90,7 +94,7 @@ Next: shorten or broaden query; use githits grep.`; it("accepts unified repository headers with focused or equal evidence", () => { expect(() => assertSearchTerminalText( - "1 result | 1 repo code hit\n\n" + + "Found 1 code result.\n\n" + "[1] github:owner/repo@abc123 packages/pkg/src/compact.ts:920-930 [repo code] - compact (function at lines 858-964)\n" + " // Merge into single summary", "search", @@ -98,7 +102,7 @@ Next: shorten or broaden query; use githits grep.`; ).not.toThrow(); expect(() => assertSearchTerminalText( - "1 result | 1 repo symbol\n\n" + + "Found 1 symbol result.\n\n" + "[1] github:owner/repo@abc123 packages/pkg/src/compact.ts:858-964 [repo symbol] - compact (function)", "search", ), @@ -107,7 +111,7 @@ Next: shorten or broaden query; use githits grep.`; it("accepts structural search evidence text", () => { const structuralText = - "1 result | 1 repo code hit\n\n" + + "Found 1 code result.\n\n" + "[1] npm:express@4.18.2 lib/client.ts:142-145 [repo code]\n" + " - class Client | lines 20-220\n" + " - method Client.send | lines 120-165\n" + @@ -124,7 +128,7 @@ Next: shorten or broaden query; use githits grep.`; it("accepts compact path matches but rejects their arbitrary numbered snippets", () => { const path = - "1 result | 1 repo code hit\n\n[1] npm:express@4.21.2 examples/route-middleware/index.js [repo code, path match]"; + "Found 1 code result.\n\n[1] npm:express@4.21.2 examples/route-middleware/index.js [repo code, path match]"; expect(() => assertSearchTerminalText(path, "search")).not.toThrow(); expect(() => assertSearchTerminalText( @@ -150,7 +154,7 @@ Next: shorten or broaden query; use githits grep.`; it("accepts documentation hits that disclose a missing source URL", () => { expect(() => assertSearchTerminalText( - "1 result | 1 docs page\n\n[1] page-1 [docs page] npm:express - source URL unavailable - README", + "Found 1 documentation result.\n\n[1] page-1 [docs page] npm:express - source URL unavailable - README", "search", ), ).not.toThrow(); @@ -159,7 +163,7 @@ Next: shorten or broaden query; use githits grep.`; it("accepts wrapped documentation and repository title tails", () => { expect(() => assertSearchTerminalText( - "2 results | 1 repo code hit, 1 docs page\n\n" + + "Found 1 code result and 1 documentation result.\n\n" + "[1] page-1 [docs page] npm:express - docs.example.com/getting-started -\n" + " A long documentation title\n\n" + "[2] npm:express@5.2.1 lib/application.js [repo code] -\n" + @@ -172,7 +176,7 @@ Next: shorten or broaden query; use githits grep.`; it("accepts a wrapped repository title without a documentation hit", () => { expect(() => assertSearchTerminalText( - "1 result | 1 repo code hit\n\n" + + "Found 1 code result.\n\n" + "[1] npm:express@5.2.1 lib/application.js [repo code] -\n" + " A long repository title", "search", @@ -182,41 +186,43 @@ Next: shorten or broaden query; use githits grep.`; it.each([ [ - "1 result\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n This payload mentions githits code read but has no locator", + "Found 1 code result.\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n This payload mentions githits code read but has no locator", ], [ - "1 result\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n githits code read 'npm:express@5.2.1' --lines 1-10", + "Found 1 code result.\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n githits code read 'npm:express@5.2.1' --lines 1-10", ], [ - "1 result\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n ordinary title\n githits code read 'npm:express@5.2.1' 'index.js'", + "Found 1 code result.\n\n[1] npm:express@5.2.1 location unavailable [repo code]\n ordinary title\n githits code read 'npm:express@5.2.1' 'index.js'", ], [ - "1 result\n\n[1] page-1 [docs page] npm:express - README\n" + + "Found 1 code result.\n\n[1] page-1 [docs page] npm:express - README\n" + " githits docs read --lines 1-10", ], [ - "1 result\n\n[1] page-1 [docs page] npm:express -\n" + + "Found 1 code result.\n\n[1] page-1 [docs page] npm:express -\n" + " README without a source locator", ], [ - "1 result\n\n[1] page ID unavailable [docs page] npm:express - docs.example.com/readme -\n" + + "Found 1 code result.\n\n[1] page ID unavailable [docs page] npm:express - docs.example.com/readme -\n" + " Wrapped title without a page locator", ], [ - "1 result\n\n[1] npm:express@5.2.1 location unavailable [repo code] -\n" + + "Found 1 code result.\n\n[1] npm:express@5.2.1 location unavailable [repo code] -\n" + " Wrapped title without a locator", ], - ["1 result\n\n[1] npm:express@5.2.1 lib/application.js [repo code] -"], [ - "1 result\n\n[1] compact - function defined at packages/pkg/src/compact.ts:858-964", + "Found 1 code result.\n\n[1] npm:express@5.2.1 lib/application.js [repo code] -", + ], + [ + "Found 1 code result.\n\n[1] compact - function defined at packages/pkg/src/compact.ts:858-964", ], [ - "1 result\n\n" + + "Found 1 code result.\n\n" + "[1] compact - function defined at packages/pkg/src/compact.ts:858-964\n" + " github:owner/repo@abc123 evidence at 920-930 [repo code]", ], [ - "1 result\n\n[1] compact - function defined at location unavailable\n" + + "Found 1 code result.\n\n[1] compact - function defined at location unavailable\n" + " github:owner/repo@abc123 evidence at 920-930 [repo code]", ], ])("rejects incomplete or prose-only hit follow-ups", (text) => { @@ -228,9 +234,12 @@ Next: shorten or broaden query; use githits grep.`; it.each([ [ "Fix", - "No results\n\n- npm:missing@1.0.0\n Fix: verify the package coordinate.", + "No results found.\n\n- npm:missing@1.0.0\n Fix: verify the package coordinate.", + ], + [ + "Try", + "No results found.\n\n- npm:missing latest\n Try: npm:missing@1.0.0", ], - ["Try", "No results\n\n- npm:missing latest\n Try: npm:missing@1.0.0"], ])( "accepts target-local %s recovery without a hit or Next", (_kind, text) => { @@ -241,10 +250,10 @@ Next: shorten or broaden query; use githits grep.`; it("accepts terminal target rows with a global rerun action", () => { expect(() => assertSearchTerminalText( - "No results | failed | 0/1 ready\n\n" + + "No results found.\nSearch failed.\n\n" + "- npm:express@4.18.2\n" + " searched: code; not found: symbols\n\n" + - "Next: rerun search later.", + "Follow-up:\n Search again later.", "search", ), ).not.toThrow(); @@ -259,7 +268,8 @@ Next: shorten or broaden query; use githits grep.`; ])("recognizes grouped target state detail: %s", (detail) => { expect(() => assertSearchTerminalText( - `No results\n\n- npm:express@4.18.2\n ${detail}\n\nNext: rerun search later.`, + `No results found.\n\n- npm:express@4.18.2\n ${detail}\n\nFollow-up: + Search again later.`, "search", ), ).not.toThrow(); @@ -274,7 +284,8 @@ Next: shorten or broaden query; use githits grep.`; ])("rejects ungrouped target state detail: %s", (detail) => { expect(() => assertSearchTerminalText( - `No results\n ${detail}\n\nNext: rerun search later.`, + `No results found.\n ${detail}\n\nFollow-up: + Search again later.`, "search", ), ).toThrow("readiness details must be grouped under a target"); @@ -326,20 +337,18 @@ Next: shorten or broaden query; use githits grep.`; it("rejects duplicate lifecycle, status, and Next lines", () => { expect(() => - assertSearchTerminalText( - `${valid}\nNo results yet | indexing | 0/1 ready`, - "search", - ), - ).toThrow("duplicate lifecycle outcome lines"); + assertSearchTerminalText(`${valid}\nNo results available yet.`, "search"), + ).toThrow("duplicate outcome lines"); expect(() => assertSearchTerminalText(`${valid}\nstatus: indexing`, "search"), ).toThrow("duplicated lifecycle status line"); expect(() => assertSearchTerminalText( - `${valid}\nNext: githits search-status other --wait 20`, + `${valid}\nFollow-up: + githits search-status other --wait 20`, "search", ), - ).toThrow("multiple Next actions"); + ).toThrow("duplicated footer section"); }); it("rejects target diagnostics and missing target grouping", () => { @@ -379,7 +388,7 @@ Next: shorten or broaden query; use githits grep.`; }); it("ignores formatter-like words and diagnostics in indented hit content", () => { - const hitText = `1 result + const hitText = `Found 1 code result. [1] npm:express@5.2.1 lib/application.js [repo code] Ready: payload text @@ -401,7 +410,7 @@ Next: shorten or broaden query; use githits grep.`; it("keeps multiline hit-body diagnostics opaque after a blank line", () => { const hitText = - "1 result | 1 repo code hit\n\n[1] npm:express@5.2.1 index.js [repo code]\n" + + "Found 1 code result.\n\n[1] npm:express@5.2.1 index.js [repo code]\n" + " First summary paragraph.\n\n" + " status: payload text\n" + " searchRef=payload text\n" + @@ -412,6 +421,25 @@ Next: shorten or broaden query; use githits grep.`; }); }); +describe("search footer structure", () => { + it("rejects a status command without its Follow-up section", () => { + expect(() => + assertSearchTerminalText( + "No results available yet.\n githits search-status ref --wait 30", + "search", + ), + ).toThrow("status action must be in Follow-up"); + }); + it("rejects out-of-order footer sections", () => { + expect(() => + assertSearchTerminalText( + "Found 1 code result.\n\nMore results:\n --offset 1\n\nRead:\n githits read target path", + "search", + ), + ).toThrow("footer sections out of order"); + }); +}); + describe("CLI transitive vulnerability smoke contract", () => { it("accepts positive results and composite MALWARE headlines", () => { const output = `Resolved dependencies diff --git a/src/commands/search.test.ts b/src/commands/search.test.ts index 1d8c0aa8..0a74ad08 100644 --- a/src/commands/search.test.ts +++ b/src/commands/search.test.ts @@ -483,7 +483,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe("No results"); + expect(output.split("\n")[0]).toBe("No results found."); expect(output).toContain( "Sources:\n - site:example.com (hosted documentation)", ); @@ -562,7 +562,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe("No results"); + expect(output.split("\n")[0]).toBe("No results found."); expect(output).toContain("- npm:express@5.1.0"); expect(output.replace(/\s+/g, " ")).toContain( "github:expressjs/express@01234567 (repository docs, requested: npm:express@5.1.0, no results)", @@ -577,7 +577,7 @@ describe("searchAction", () => { expect(output).not.toContain("Try a shorter or broader query"); expect(output).not.toContain("Run again with a larger --wait"); expect(output).not.toContain("Evidence may change."); - expect(output).toContain("Next: search again later."); + expect(output).toContain("Search again later."); consoleSpy.mockRestore(); }); @@ -602,7 +602,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe("No results"); + expect(output.split("\n")[0]).toBe("No results found."); expect(output).toContain("- npm:express@5.1.0"); expect(output.replace(/\s+/g, " ")).toContain( "github:expressjs/express@01234567 (repository docs, requested: npm:express@5.1.0, no results)", @@ -652,7 +652,7 @@ describe("searchAction", () => { expect(output.replace(/\s+/g, " ")).toContain( "github:expressjs/express@01234567 (repository docs, requested: npm:express@5.1.0, no results)", ); - expect(output).toContain("Next: search again later."); + expect(output).toContain("Search again later."); consoleSpy.mockRestore(); }); @@ -699,7 +699,9 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output).toContain("Next: shorten or broaden site query."); + expect(output).toContain( + "Follow-up:\n Try: shorten or broaden site query.", + ); expect(output).not.toContain("search another source"); consoleSpy.mockRestore(); }); @@ -753,7 +755,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe("1 result | 1 docs page"); + expect(output.split("\n")[0]).toBe("Found 1 documentation result."); expect(output.replace(/\s+/g, " ")).toContain( "Sources: - github:expressjs/express@01234567 (repository docs, requested: npm:express@5.1.0, no results) - site:expressjs.com/en/guide (hosted documentation, requested: npm:express@5.1.0)", ); @@ -1015,7 +1017,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe("1 result | 1 repo code hit"); + expect(output.split("\n")[0]).toBe("Found 1 code result."); expect(output).toContain( "[1] npm:express@4.18.2 lib/router/index.js:42-57 [repo code] - router middleware", ); @@ -1052,11 +1054,9 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "No result snapshot yet | indexing | 0/1 ready", - ); + expect(output.split("\n")[0]).toBe("No results available yet."); expect(output).toContain( - "Next: githits search-status search-ref-123 --wait 30", + "Follow-up:\n githits search-status search-ref-123 --wait 30", ); consoleSpy.mockRestore(); }); @@ -1163,18 +1163,18 @@ describe("searchAction", () => { const initial = String(consoleSpy.mock.calls[0]?.[0]); expect(initial).toBe( [ - "No results yet | indexing | 0/1 ready", + "No results available yet.", "", "- npm:n8n -> 2.36.7", " indexing: code, repository docs; available: n8n.io docs (1,480 pages; capped);", - " indexed: versions 2.26.9, 2.26.5, 2.23.2 +2, refs HEAD, master", + " indexed: versions 2.26.9, 2.26.5, 2.23.2 (+2 more), refs HEAD, master", "", - "Next: githits search-status n8n-search-ref --wait 30", + "Follow-up:\n githits search-status n8n-search-ref --wait 30", ].join("\n"), ); - expect(initial.match(/^No results yet/gm)).toHaveLength(1); + expect(initial.match(/^No results available yet/gm)).toHaveLength(1); expect(initial.match(/^Search /gm)).toBeNull(); - expect(initial.match(/^Next:/gm)).toHaveLength(1); + expect(initial.match(/^Follow-up:/gm)).toHaveLength(1); expect(initial.match(/n8n-search-ref/g)).toHaveLength(1); expect(initial).not.toContain("search_status search_ref="); expect(initial).not.toContain("Warning:"); @@ -1204,7 +1204,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe("No results yet | indexing | 0/1 ready"); + expect(output.split("\n")[0]).toBe("No results available yet."); expect(output).toContain("- npm:express@5.1.0"); expect(output.replace(/\s+/g, " ")).toContain( "github:expressjs/express@01234567 (repository docs, requested: npm:express@5.1.0, no results)", @@ -1236,9 +1236,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "1 result | 1 repo code hit | deferred | 0/1 ready", - ); + expect(output.split("\n")[0]).toBe("Found 1 code result."); expect(output).toContain("- npm:express@4.18.2"); expect(output).toContain( "[1] npm:express@4.18.2 lib/router/index.js:42-57 [repo code] - router middleware", @@ -1248,7 +1246,7 @@ describe("searchAction", () => { expect(output).not.toContain("githits search-status"); expect(output).not.toContain("re-run with the searchRef"); expect(output).not.toContain("still indexing"); - expect(output).not.toContain("No results\n"); + expect(output).not.toContain("No results found.\n"); expect(output).not.toContain("Indexing/search still in progress"); consoleSpy.mockRestore(); }); @@ -1275,9 +1273,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "1 result | 1 repo code hit | status unknown | 0/1 ready", - ); + expect(output.split("\n")[0]).toBe("Found 1 code result."); expect(output).toContain("- npm:express@4.18.2"); expect(output).toContain( "[1] npm:express@4.18.2 lib/router/index.js:42-57 [repo code] - router middleware", @@ -1287,7 +1283,7 @@ describe("searchAction", () => { expect(output).not.toContain("githits search-status"); expect(output).not.toContain("re-run with the searchRef"); expect(output).not.toContain("still indexing"); - expect(output).not.toContain("No results"); + expect(output).not.toContain("No results found."); expect(output).not.toContain("terminal"); consoleSpy.mockRestore(); }); @@ -1330,7 +1326,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe("No results yet | indexing | 0/1 ready"); + expect(output.split("\n")[0]).toBe("No results available yet."); expect(output).toContain("- site:example.com"); expect(output).toContain("indexing: site:example.com docs"); expect(output).toContain("incompatible filter (docs): language"); @@ -1338,7 +1334,7 @@ describe("searchAction", () => { expect(output).not.toContain("Try: site:docs.example.com"); expect(output).toContain("+more"); expect(output).toContain( - "Next: githits search-status search-ref-site --wait 30", + "Follow-up:\n githits search-status search-ref-site --wait 30", ); consoleSpy.mockRestore(); }); @@ -1542,7 +1538,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output).toContain("1 result"); + expect(output).toContain("Found 1 code result."); expect(output).not.toContain("Search still in progress"); expect(output).not.toContain("Indexing/search still in progress"); expect(output).not.toContain("Warning: Search completed"); @@ -1600,7 +1596,7 @@ describe("searchAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output).toContain("1 result"); + expect(output).toContain("Found 1 code result."); expect(output).toContain( "- npm:express@4.18.2\n using: provisional snapshot; searched: code (provisional)", ); @@ -1666,7 +1662,7 @@ describe("searchAction", () => { expect(output).toMatch(/indexed:\s+refs\s+master/); expect(output).not.toContain("Evidence:"); expect(output).not.toContain("Indexed alternatives:"); - expect(output).not.toContain("Next: githits search-status"); + expect(output).not.toContain("Follow-up:\n githits search-status"); consoleSpy.mockRestore(); }); @@ -2087,12 +2083,10 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "No result snapshot yet | searching | 1/1 ready", - ); + expect(output.split("\n")[0]).toBe("No results available yet."); expect(output).not.toContain("Search search-ref-123 |"); expect(output).toContain( - "Next: githits search-status search-ref-123 --wait 30", + "Follow-up:\n githits search-status search-ref-123 --wait 30", ); consoleSpy.mockRestore(); }); @@ -2124,13 +2118,11 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "No result snapshot yet | indexing | 0/1 ready", - ); + expect(output.split("\n")[0]).toBe("No results available yet."); expect(output).toContain("- site:example.com"); expect(output).not.toContain("Search search-ref-stale |"); expect(output).toContain( - "Next: githits search-status search-ref-stale --wait 30", + "Follow-up:\n githits search-status search-ref-stale --wait 30", ); consoleSpy.mockRestore(); }); @@ -2181,7 +2173,7 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe("No results yet | indexing | 0/1 ready"); + expect(output.split("\n")[0]).toBe("No results available yet."); expect(output).toContain("- site:example.com"); expect(output).toContain( "Sources:\n - site:example.com (hosted documentation)", @@ -2190,7 +2182,7 @@ describe("searchStatusAction", () => { expect(output).not.toContain("Try: site:docs.example.com"); expect(output).not.toContain("Search search-ref-site |"); expect(output).toContain( - "Next: githits search-status search-ref-site --wait 30", + "Follow-up:\n githits search-status search-ref-site --wait 30", ); consoleSpy.mockRestore(); }); @@ -2286,16 +2278,14 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "No result snapshot yet | indexing | 0/1 ready", - ); + expect(output.split("\n")[0]).toBe("No results available yet."); expect(output).toContain( "- github:expressjs/express@refs/heads/master -> master", ); expect(output).toContain("indexed: refs master"); expect(output).not.toContain("Search search-ref-123 |"); expect(output).toContain( - "Next: githits search-status search-ref-123 --wait 30", + "Follow-up:\n githits search-status search-ref-123 --wait 30", ); consoleSpy.mockRestore(); }); @@ -2320,11 +2310,9 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "No result snapshot | timeout | 0/1 ready", - ); + expect(output.split("\n")[0]).toBe("No result snapshot available."); expect(output).not.toContain("Search search-ref-timeout |"); - expect(output).toContain("Next: search again later."); + expect(output).toContain("Search again later."); expect(output).not.toContain("longer wait"); expect(output).not.toContain("Search still in progress."); consoleSpy.mockRestore(); @@ -2379,16 +2367,14 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "1 result | 1 repo code hit | deferred | 1/2 ready", - ); + expect(output.split("\n")[0]).toBe("Found 1 code result."); expect(output).toContain( "[1] npm:express@4.18.2 lib/router/index.js:42-57 [repo code] - router middleware", ); expect(output).not.toContain("Search ref-deferred |"); expect(output).toContain("For updated results, search again."); expect(output).not.toContain("githits search-status"); - expect(output).not.toContain("No results"); + expect(output).not.toContain("No results found."); expect(output).not.toContain("Indexing/search still in progress"); consoleSpy.mockRestore(); }); @@ -2418,16 +2404,14 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "1 result | 1 repo code hit | status unknown | 0/1 ready", - ); + expect(output.split("\n")[0]).toBe("Found 1 code result."); expect(output).toContain( "[1] npm:express@4.18.2 lib/router/index.js:42-57 [repo code] - router middleware", ); expect(output).not.toContain("Search ref-future |"); expect(output).toContain("For updated results, search again."); expect(output).not.toContain("githits search-status"); - expect(output).not.toContain("No results"); + expect(output).not.toContain("No results found."); expect(output).not.toContain("Indexing/search still in progress"); expect(output).not.toContain("terminal"); consoleSpy.mockRestore(); @@ -2451,12 +2435,10 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "No result snapshot | deferred | 0/1 ready", - ); + expect(output.split("\n")[0]).toBe("No result snapshot available."); expect(output).not.toContain("Search ref-deferred-empty |"); - expect(output).toContain("Next: search again later."); - expect(output).not.toContain("No results"); + expect(output).toContain("Search again later."); + expect(output).not.toContain("No results found."); expect(output).not.toContain("Indexing/search still in progress"); expect(output).not.toContain("githits search-status"); consoleSpy.mockRestore(); @@ -2482,9 +2464,7 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe( - "No result snapshot | failed | 0/1 ready", - ); + expect(output.split("\n")[0]).toBe("No result snapshot available."); expect(output).not.toContain("Search still in progress."); consoleSpy.mockRestore(); }); @@ -2555,7 +2535,7 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe("No results"); + expect(output.split("\n")[0]).toBe("No results found."); expect(output).toContain("- npm:express@5.1.0"); expect(output.replace(/\s+/g, " ")).toContain( "github:expressjs/express@01234567 (repository docs, requested: npm:express@5.1.0, no results)", @@ -2564,7 +2544,7 @@ describe("searchStatusAction", () => { /available: expressjs\.com\/en\/guide docs \(120 pages; partial\)/, ); expect(output).not.toContain("Evidence may change."); - expect(output).toContain("Next: search again later."); + expect(output).toContain("Search again later."); expect(output).not.toContain("githits search-status"); expect(output).not.toContain("Search completed"); expect(output).not.toContain("re-run with the searchRef"); @@ -2596,7 +2576,7 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output.split("\n")[0]).toBe("No results"); + expect(output.split("\n")[0]).toBe("No results found."); expect(output).toContain("- npm:express@5.1.0"); expect(output.replace(/\s+/g, " ")).toContain( "github:expressjs/express@01234567 (repository docs, requested: npm:express@5.1.0, no results)", @@ -2648,7 +2628,9 @@ describe("searchStatusAction", () => { ); const output = String(consoleSpy.mock.calls[0]?.[0]); - expect(output).toContain("Next: shorten or broaden site query."); + expect(output).toContain( + "Follow-up:\n Try: shorten or broaden site query.", + ); expect(output).not.toContain("search another source"); consoleSpy.mockRestore(); }); diff --git a/src/tools/search-parity.test.ts b/src/tools/search-parity.test.ts index 13768c22..8bafbc00 100644 --- a/src/tools/search-parity.test.ts +++ b/src/tools/search-parity.test.ts @@ -431,9 +431,11 @@ describe("search parity", () => { expect(results[1]).not.toHaveProperty("summary"); const text = await cliTextForOutcome(outcome); const mcpText = await mcpTextForOutcome(outcome); - expect(text).toBe(mcpText); - expect(text).not.toContain("githits read "); - expect(mcpText).not.toContain("read target="); + expect(text.split("\n\nRead:")[0]).toBe(mcpText.split("\n\nRead:")[0]); + expect(text).toContain("githits read "); + expect(mcpText).toContain("read target="); + expect(text.split("\n\nRead:")[0]).not.toContain("githits read "); + expect(mcpText.split("\n\nRead:")[0]).not.toContain("read target="); expect(text).toContain( "lib/client.ts:120-165 [repo code, candidate; indexed: path]", ); @@ -478,13 +480,15 @@ describe("search parity", () => { }); }); - it("PARITY-TEXT-FORMATTER: shared evidence without read commands", async () => { + it("PARITY-TEXT-FORMATTER: shared evidence with native read commands", async () => { const outcome = evidenceOutcome(); const cli = await cliTextForOutcome(outcome); const mcp = await mcpTextForOutcome(outcome); - expect(cli).toBe(mcp); - expect(cli).not.toContain("githits read "); - expect(mcp).not.toContain("read target="); + expect(cli.split("\n\nRead:")[0]).toBe(mcp.split("\n\nRead:")[0]); + expect(cli).toContain("githits read "); + expect(mcp).toContain("read target="); + expect(cli.split("\n\nRead:")[0]).not.toContain("githits read "); + expect(mcp.split("\n\nRead:")[0]).not.toContain("read target="); }); it("PARITY-STRUCTURAL-JSON: CLI === MCP and preserves structural evidence", async () => { @@ -534,9 +538,11 @@ describe("search parity", () => { const cli = await cliTextForOutcome(outcome); const mcp = await mcpTextForOutcome(outcome); - expect(cli).toBe(mcp); - expect(cli).not.toContain("githits read "); - expect(mcp).not.toContain("read target="); + expect(cli.split("\n\nRead:")[0]).toBe(mcp.split("\n\nRead:")[0]); + expect(cli).toContain("githits read "); + expect(mcp).toContain("read target="); + expect(cli.split("\n\nRead:")[0]).not.toContain("githits read "); + expect(mcp.split("\n\nRead:")[0]).not.toContain("read target="); expect(cli).toContain( "[1] npm:express@4.18.2 lib/client.ts:142-145 [repo code]", ); @@ -550,7 +556,7 @@ describe("search parity", () => { expect(cli).toContain(" 145 | }"); expect(cli).not.toContain("legacy summary must remain in JSON"); expect(cli).not.toContain("Read context"); - expect(cli).not.toContain("read target="); + expect(cli.split("\n\nRead:")[0]).not.toContain("read target="); }); }); @@ -807,9 +813,9 @@ describe("usable snapshot presentation parity", () => { expect(text.replace(/\s+/g, " ")).toContain( "indexing, estimated total: 100-120s, committed 2026-10-05, observed HEAD", ); - expect(text).toContain("Next: use these hits"); - expect(text).not.toContain("Next: search_status"); - expect(text).not.toContain("Next: githits search-status"); + expect(text).toContain("Use these results"); + expect(text).not.toContain("Follow-up:\n search_status"); + expect(text).not.toContain("Follow-up:\n githits search-status"); expect(text).toContain("If you need current HEAD"); } expect(mcp).toContain( From 899fabc32c31e081a3b89b2ff9b4c831d10c0e1e Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Wed, 7 Oct 2026 10:15:14 +0300 Subject: [PATCH 6/8] fix: scope empty grep cursor pages accurately Keep empty outcomes page-scoped whenever a continuation cursor exists, independently of coverage warnings and terminal omissions. Add regressions for skipped content and unsupported targets, strengthen lifecycle assertions, and record review closure. --- .../search-snapshot-presentation.md | 12 ++++++++- docs/plans/search-grep-headers-footers.md | 18 +++++++++++++ packages/mcp/src/shared/grep-response.test.ts | 26 +++++++++++++++++++ packages/mcp/src/shared/grep-text.ts | 8 +----- .../src/shared/unified-search-text.test.ts | 7 ++--- 5 files changed, 60 insertions(+), 11 deletions(-) diff --git a/docs/implementation/search-snapshot-presentation.md b/docs/implementation/search-snapshot-presentation.md index 606345ea..deed4d8d 100644 --- a/docs/implementation/search-snapshot-presentation.md +++ b/docs/implementation/search-snapshot-presentation.md @@ -576,4 +576,14 @@ zero tool calls, no final/isolation artifacts, unknown usage. No comprehension claim follows. Internal review accepted and closed an overall grep traversal warning hidden by a sibling cursor and two stale documentation paragraphs. The eight-file closure passes 244 tests, zero failures and 913 assertions; no -new infrastructure or major deferred item. External review result follows. +new infrastructure or major deferred item. External round 1 accepted a cursor-page +headline inconsistency: cursor presence +now controls the page qualifier independently of coverage and terminal omissions, +while attributed/global warnings retain the limitation. Headline regressions cover +skips, failed/partial overall traversal and non-retryable omissions with cursors. + +The nine-file finding closure passes 354 tests, zero failures and 1,353 +assertions. Internal review of the revised delta is clean: cursor presence scopes +zero-hit pages, while strict source-level exhaustive claims remain unchanged. +Typecheck, both builds and source/built CLI/MCP smoke checks pass after this +correction. diff --git a/docs/plans/search-grep-headers-footers.md b/docs/plans/search-grep-headers-footers.md index c1f2fed7..f0906615 100644 --- a/docs/plans/search-grep-headers-footers.md +++ b/docs/plans/search-grep-headers-footers.md @@ -516,3 +516,21 @@ traversal branches, target gap/exhaustive predicates, parser enum/cursor contrac zero/hit pages and related documentation. Added both statuses with zero/hit pages; no service/state change or speculative mechanism. Two stale grep-doc outcome paragraphs now match current prose. Internal revised-delta closure is clean; 244 tests/zero failures/913 assertions across eight affected files prove the fix. Closure typecheck, both builds and source/built CLI/MCP smoke checks passed. + + +External round 1: direction sound, not clean (one code finding, one doc nit). +F1 accepted: empty cursor pages gated their page qualifier on coverage/omission +classification, so ordinary skipped-file + cursor and terminal-omission + cursor +pages could sound exhaustive. Root invariant is that a returned cursor scopes the +outcome to this page independently of missing coverage; warnings own limitations. +Removed that gate. Closure scan covers all empty grep branches, source exhaustive +qualifiers, target and overall failure warnings, retryable/non-retryable omissions, +normal/unvisited pagination and search/status's corresponding page qualifier. +Tests assert headlines for skipped-file and failed/partial cursor pages, plus a +non-retryable-omission cursor page. F2 accepted: replaced a dangling permanent-doc +review placeholder with the verified closure. Also restored explicit lifecycle +sentence assertions where a migrated test left an unused label. No machinery or +scope expansion. Nine-file closure passes 354 tests, zero failures and 1,353 +assertions. Internal revised-delta closure is clean. Typecheck, both builds, and +source/built CLI/MCP smoke checks pass after the external correction; external +round 2 pending. diff --git a/packages/mcp/src/shared/grep-response.test.ts b/packages/mcp/src/shared/grep-response.test.ts index b88e4321..5ce23ab2 100644 --- a/packages/mcp/src/shared/grep-response.test.ts +++ b/packages/mcp/src/shared/grep-response.test.ts @@ -463,6 +463,7 @@ describe("grep page and omission outcomes", () => { nextCursor: "cursor", }), ); + expect(output.split("\n")[0]).toBe("No matches on this page."); expect(output).toContain("Repository npm:x:"); expect(output).toContain("Skipped 1 binary file(s)."); expect(output).toContain("--cursor 'cursor'"); @@ -499,3 +500,28 @@ describe("overall grep traversal limitations", () => { }, ); }); + +it("scopes an empty cursor page independently of non-retryable omissions", () => { + const output = formatGrepText( + result({ + hits: [], + totalMatches: 0, + traversal: "RESUMABLE_LIMIT", + nextCursor: "cursor", + unavailableTargets: [ + { + inputIndex: 0, + target: "npm:unsupported", + reason: "unsupported", + retryable: false, + progressRef: null, + suggestedSiteTargets: null, + }, + ], + }), + ); + expect(output.split("\n")[0]).toBe("No matches on this page."); + expect(output).toContain("Omitted:\n - npm:unsupported (unsupported)"); + expect(output).toContain("--cursor 'cursor'"); + expect(output).not.toContain("To retry omitted targets"); +}); diff --git a/packages/mcp/src/shared/grep-text.ts b/packages/mcp/src/shared/grep-text.ts index 93384057..2c54627a 100644 --- a/packages/mcp/src/shared/grep-text.ts +++ b/packages/mcp/src/shared/grep-text.ts @@ -74,16 +74,10 @@ export function formatGrepText( : kinds.has("GrepSiteHit") ? `page${groups.length === 1 ? "" : "s"}` : `file${groups.length === 1 ? "" : "s"}`; - const pageGap = - result.targets.some(hasPageCoverageGap) || - (!omissionsOnly && - !["COMPLETE", "RESUMABLE_LIMIT"].includes(result.traversal)); prose( result.hits.length ? `Found ${result.totalMatches} match${result.totalMatches === 1 ? "" : "es"} on ${matchingLines} line${matchingLines === 1 ? "" : "s"} in ${groups.length} ${noun}.` - : result.nextCursor && - !pageGap && - (result.unavailableTargets.length === 0 || retryableOmissionsOnly) + : result.nextCursor ? retryableOmissionsOnly ? "No matches available yet on this page." : "No matches on this page." diff --git a/packages/mcp/src/shared/unified-search-text.test.ts b/packages/mcp/src/shared/unified-search-text.test.ts index 17580568..17046457 100644 --- a/packages/mcp/src/shared/unified-search-text.test.ts +++ b/packages/mcp/src/shared/unified-search-text.test.ts @@ -2311,9 +2311,9 @@ describe("renderUnifiedSearchSuccess", () => { }); it.each([ - ["PENDING", "preparing"], - ["INDEXING", "indexing"], - ["SEARCHING", "searching"], + ["PENDING", "Search is preparing requested sources."], + ["INDEXING", "Requested sources are still indexing."], + ["SEARCHING", "Search is still running."], ] as const)( "keeps %s lifecycle distinct without a snapshot", (status, label) => { @@ -2328,6 +2328,7 @@ describe("renderUnifiedSearchSuccess", () => { }), ); expect(firstLine(text)).toBe("No results available yet."); + expect(text).toContain(label); }, ); From 384e9c24bec402a67180db2e353e2ad6d490d746 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Wed, 7 Oct 2026 10:17:26 +0300 Subject: [PATCH 7/8] test: prove cursor headlines beside traversal limits Assert page-scoped zero-hit and retained-hit headlines for failed and partial overall traversal, group omission coverage with related tests, and record external review closure. --- .../search-snapshot-presentation.md | 10 +++- docs/plans/search-grep-headers-footers.md | 10 ++++ packages/mcp/src/shared/grep-response.test.ts | 54 ++++++++++--------- 3 files changed, 47 insertions(+), 27 deletions(-) diff --git a/docs/implementation/search-snapshot-presentation.md b/docs/implementation/search-snapshot-presentation.md index deed4d8d..4cbdc149 100644 --- a/docs/implementation/search-snapshot-presentation.md +++ b/docs/implementation/search-snapshot-presentation.md @@ -577,8 +577,7 @@ claim follows. Internal review accepted and closed an overall grep traversal warning hidden by a sibling cursor and two stale documentation paragraphs. The eight-file closure passes 244 tests, zero failures and 913 assertions; no new infrastructure or major deferred item. External round 1 accepted a cursor-page -headline inconsistency: cursor presence -now controls the page qualifier independently of coverage and terminal omissions, +headline inconsistency: cursor presence now controls the page qualifier independently of coverage and terminal omissions, while attributed/global warnings retain the limitation. Headline regressions cover skips, failed/partial overall traversal and non-retryable omissions with cursors. @@ -587,3 +586,10 @@ assertions. Internal review of the revised delta is clean: cursor presence scope zero-hit pages, while strict source-level exhaustive claims remain unchanged. Typecheck, both builds and source/built CLI/MCP smoke checks pass after this correction. + +External round 2 confirmed the behavior but found a missing headline assertion +in the overall failed/partial fixture. Both zero/hit shapes now assert the +headline for both overall states; warning/cursor assertions remain. The omission +regression is grouped with its related cases. This closure changes tests only. +Focused closure passes 19 tests, zero failures and 118 assertions. Internal +review of the revised delta is clean. diff --git a/docs/plans/search-grep-headers-footers.md b/docs/plans/search-grep-headers-footers.md index f0906615..75398cdf 100644 --- a/docs/plans/search-grep-headers-footers.md +++ b/docs/plans/search-grep-headers-footers.md @@ -534,3 +534,13 @@ scope expansion. Nine-file closure passes 354 tests, zero failures and 1,353 assertions. Internal revised-delta closure is clean. Typecheck, both builds, and source/built CLI/MCP smoke checks pass after the external correction; external round 2 pending. + +External round 2: direction sound; R1 behavior and closure correct. Low finding +accepted: the overall failed/partial fixture asserted warning/cursor but omitted +the headline assertion claimed above. Root class is evidence that does not prove +a documentation claim. Scanned the new skip/omission/overall fixture assertions +against those claims; added explicit zero/hit headlines for both overall states. +Moved the omission test into the related describe block (accepted test-organization +nit). Production is unchanged. Focused `bun test packages/mcp/src/shared/grep-response.test.ts` +passes 19 tests, zero failures and 118 assertions; scoped Biome and diff checks +pass. Internal full revised-delta closure is clean. Round 3 pending. diff --git a/packages/mcp/src/shared/grep-response.test.ts b/packages/mcp/src/shared/grep-response.test.ts index 5ce23ab2..cf572fee 100644 --- a/packages/mcp/src/shared/grep-response.test.ts +++ b/packages/mcp/src/shared/grep-response.test.ts @@ -469,6 +469,30 @@ describe("grep page and omission outcomes", () => { expect(output).toContain("--cursor 'cursor'"); expect(output).not.toContain("(no results)"); }); + it("scopes an empty cursor page independently of non-retryable omissions", () => { + const output = formatGrepText( + result({ + hits: [], + totalMatches: 0, + traversal: "RESUMABLE_LIMIT", + nextCursor: "cursor", + unavailableTargets: [ + { + inputIndex: 0, + target: "npm:unsupported", + reason: "unsupported", + retryable: false, + progressRef: null, + suggestedSiteTargets: null, + }, + ], + }), + ); + expect(output.split("\n")[0]).toBe("No matches on this page."); + expect(output).toContain("Omitted:\n - npm:unsupported (unsupported)"); + expect(output).toContain("--cursor 'cursor'"); + expect(output).not.toContain("To retry omitted targets"); + }); it("explains unspecified readiness when it is not an unvisited continuation", () => { const output = formatGrepText( result({ targets: [{ ...target, readiness: "UNSPECIFIED" }] }), @@ -490,6 +514,11 @@ describe("overall grep traversal limitations", () => { nextCursor: "sibling cursor", }), ); + expect(output.split("\n")[0]).toBe( + hits.length + ? "Found 1 match on 1 line in 1 file." + : "No matches on this page.", + ); expect(output).toContain( "Some requested content could not be searched.", ); @@ -500,28 +529,3 @@ describe("overall grep traversal limitations", () => { }, ); }); - -it("scopes an empty cursor page independently of non-retryable omissions", () => { - const output = formatGrepText( - result({ - hits: [], - totalMatches: 0, - traversal: "RESUMABLE_LIMIT", - nextCursor: "cursor", - unavailableTargets: [ - { - inputIndex: 0, - target: "npm:unsupported", - reason: "unsupported", - retryable: false, - progressRef: null, - suggestedSiteTargets: null, - }, - ], - }), - ); - expect(output.split("\n")[0]).toBe("No matches on this page."); - expect(output).toContain("Omitted:\n - npm:unsupported (unsupported)"); - expect(output).toContain("--cursor 'cursor'"); - expect(output).not.toContain("To retry omitted targets"); -}); From 9d9c72d4211380d25e8c1df0e5b27cbbe82110f2 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Wed, 7 Oct 2026 10:20:03 +0300 Subject: [PATCH 8/8] docs: retire completed search and grep output plan Keep the verified output contract, evidence and validation limits in implementation documentation. Remove the completed working plan and selected backlog entry after clean internal and Claude review. --- .../search-snapshot-presentation.md | 32 +- docs/plans/open-backlog.md | 5 - docs/plans/search-grep-headers-footers.md | 546 ------------------ 3 files changed, 10 insertions(+), 573 deletions(-) delete mode 100644 docs/plans/search-grep-headers-footers.md diff --git a/docs/implementation/search-snapshot-presentation.md b/docs/implementation/search-snapshot-presentation.md index 4cbdc149..dc4795dc 100644 --- a/docs/implementation/search-snapshot-presentation.md +++ b/docs/implementation/search-snapshot-presentation.md @@ -532,8 +532,9 @@ Healthy searches offer one exact read example without use-now prose. When a wait is offered beside usable hits, the read comes first with short use-now advice; the wait remains conditional. Readless usable results put `Use these results now.` first in Follow-up. Grep templates follow all matches; CLI templates include -`githits`. Exact operands are never split by prose wrapping, and ANSI changes -only emphasis. Empty outputs omit read advice. +`githits`. Standalone read, pagination and status actions remain unwrapped; +grep omitted-target retry remains wrapped guidance, as in the approved copy. +ANSI changes only emphasis. Empty outputs omit read advice. Pagination always lives in More results, independently of lifecycle. Search/status instruct repeating the original search with the exact next offset, preserving @@ -573,23 +574,10 @@ adds actionable read/pagination footers the baseline omitted. These are output bytes, not tokens, latency or proof of agent quality. Targeted Claude agent:e2e search-investigation and grep-mixed-docs remained blocked by `Not logged in`: zero tool calls, no final/isolation artifacts, unknown usage. No comprehension -claim follows. Internal review accepted and closed an overall grep traversal -warning hidden by a sibling cursor and two stale documentation paragraphs. -The eight-file closure passes 244 tests, zero failures and 913 assertions; no -new infrastructure or major deferred item. External round 1 accepted a cursor-page -headline inconsistency: cursor presence now controls the page qualifier independently of coverage and terminal omissions, -while attributed/global warnings retain the limitation. Headline regressions cover -skips, failed/partial overall traversal and non-retryable omissions with cursors. - -The nine-file finding closure passes 354 tests, zero failures and 1,353 -assertions. Internal review of the revised delta is clean: cursor presence scopes -zero-hit pages, while strict source-level exhaustive claims remain unchanged. -Typecheck, both builds and source/built CLI/MCP smoke checks pass after this -correction. - -External round 2 confirmed the behavior but found a missing headline assertion -in the overall failed/partial fixture. Both zero/hit shapes now assert the -headline for both overall states; warning/cursor assertions remain. The omission -regression is grouped with its related cases. This closure changes tests only. -Focused closure passes 19 tests, zero failures and 118 assertions. Internal -review of the revised delta is clean. +claim follows. The revised cursor/coverage closure passes 354 focused tests, +zero failures and 1,353 assertions; the final headline-assertion closure passes +19 tests, zero failures and 118 assertions. Typecheck, both builds and +source/built CLI/MCP smoke checks pass after the production correction. +Internal review and fresh Claude Opus 5.5 review, including its final +fresh-context check, are clean for implementation commit 384e9c2. No major +deferred item or new infrastructure was introduced. diff --git a/docs/plans/open-backlog.md b/docs/plans/open-backlog.md index 13d3f865..f6ba7235 100644 --- a/docs/plans/open-backlog.md +++ b/docs/plans/open-backlog.md @@ -1,10 +1,5 @@ # Open backlog -## Unify search and grep headers and footers - -Selected for design on 2026-10-06. The concrete design and implementation -acceptance criteria now live in [search-grep-headers-footers.md](search-grep-headers-footers.md). - ## Optimize overall tool routing and investigation instructions User deferred this follow-up until after unified MCP grep adoption on diff --git a/docs/plans/search-grep-headers-footers.md b/docs/plans/search-grep-headers-footers.md deleted file mode 100644 index 75398cdf..00000000 --- a/docs/plans/search-grep-headers-footers.md +++ /dev/null @@ -1,546 +0,0 @@ -# Search and grep headers and footers - -## Status and destination - -- Overall: **IN PROGRESS — implementation authorized**. -- Phase 1: **IN PROGRESS** — one implementation increment makes search/status and - grep headers and footers consistent on CLI and local/published MCP package text. -- Product decisions: **none open**. The user approved the plain-language - direction, constrained repeated prose on 2026-10-07, and invoked $implement. - These instructions settle the previously proposed wording and concision rules. -- Dependencies: merged PR #454 (`c71ffb5`), current main `bc295b3`, existing source/preparation facts, - existing read actions, search offset and grep cursor contracts. - -Readers should immediately see what this page returned, what is preparing, and -how to read, continue or obtain updated results. Search and grep retain their different evidence -counts and continuation semantics within one visible anatomy. This is one -bounded PR with product changes, not a planning-only PR. Implementation is authorized; no merge or release. - -## Verified baseline - -Authenticated dev captures on 2026-10-06 exercised all four commands with CLI -`npm:express@2.3.10` and local MCP `npm:express@2.3.11`. Both were registry-confirmed -and unindexed during the calls. The durable record is -[search-snapshot-presentation.md](../implementation/search-snapshot-presentation.md#shared-sourcepreparation-boundary-2026-10-06). -Final full text and metadata are preserved locally in -`/tmp/shared-source-final-outputs.md` and `/tmp/shared-source-final-metadata.md`; -these files are supplemental evidence, not implementation dependencies. - -Search returned a usable documentation result but started with: - -```text -1 partial result | 1 docs page | indexing | 0/1 ready | next_offset=1 -``` - -Grep returned usable documentation while the same repository was preparing: - -```text -1 match in 1 line across 1 page; more available -``` - -Both already display Sources and Preparing. Search includes requested aliases -on its hosted-doc source when needed; grep scopes and source attribution have -separate semantics. Those facts must survive; equal-looking strings are not -proof of equal facts. - -Main was refreshed during planning to `bc295b3` (release0.27.0 and an -example-source tool). The diff from `c71ffb5` changes none of the inspected -search/grep formatters, projection/response, status API or smoke validators. -These selected contracts therefore remain current; implementation starts from -then-current main rather than requiring this planning branch as a code base. - -Code inspection on the merged baseline established: - -- `unified-search-text.ts` owns the outcome and action rendering; the status - renderer delegates to it. `UnifiedSearchTextResult.nextOffset` is already - available for both initial and retained-status pages. -- `progress.targetsReady/targetsTotal` is target readiness, not a count of usable - hits or ready documentation contributors. Remove this ambiguous text shorthand; - JSON retains the counts and per-scope context retains readiness facts. -- Search currently puts an offset only in the headline; it has no dedicated - pagination footer. Its lifecycle action union describes polling/retry, not - pagination. A healthy completed result has action `none`, so it also omits the - existing first-hit read example. -- `search` accepts `--offset` / `offset`; `search-status` / `search_status` does - not accept an offset. Status query echo omits original compile/filter options. - Never reconstruct a complete filtered search from a status result or hit label. -- Grep's `totalMatches` is occurrences on this page. The formatter derives unique - matching lines and files/pages. `nextCursor` is a complete backend operand; - existing validation rejects resumable traversal without a cursor. -- Grep `hasCoverageGap` treats any traversal other than COMPLETE as a gap. - `RESUMABLE_LIMIT` is normal pagination, so this predicate alone cannot drive a - new incomplete-coverage headline. Keep exhaustive/no-match decisions intact. -- Search availability distinguishes backend partial results from active interim - snapshots. Completed partial responses also exist; completion must not erase - that fact. Terminal and unknown lifecycle states remain independently visible. -- Footer read operands come from `readTarget` or grep's existing native templates. - `indexing-wait.ts` and `discovery-indexing-wait.ts` retain native wait policy. -- Search's bounded alternative suffix is ` +N`; read recovery already uses - `(+N more)`. Category names such as versions versus versions/refs reflect - different facts and must not be normalized into false equivalence. - -No performance claim or changed computation path is proposed. A benchmark is -not needed for choosing these text layouts; no optimization is part of scope. - -## Selected output design - -### Header — revised proposal after user feedback - -The earlier pipe-separated design is superseded. It counted one documentation -hit twice (`1 result | 1 docs page`) and used `partial` without saying what was -missing. The user contested this on 2026-10-06; do not implement that shape. - -Lead with one plain sentence describing this returned page. Count each search -result once, either as a known kind or in a mixed-kind breakdown; do not add a -redundant total. Documentation results are returned hits, not a newly invented -count of unique URLs (multiple sections can come from one page). - -```text -Found 1 documentation result. -Found 2 code results and 1 documentation result. -Found 4 matches on 3 lines in 2 files. -``` - -Search labels should distinguish repository documentation, hosted documentation -and symbols when supplied by existing hit kinds; unknown kinds retain a plain -result count rather than acquiring an invented classification. Grep's matches, -lines and files/pages are different quantities, so retain their useful relation -in the sentence. Both start with Found and use normal pluralization. - -Do not append `partial`, `interim`, readiness fractions or pagination parameters. -More results belongs solely in its footer. Preparation and limitations are -explained in sentences or their existing attributed sections, not compact flags. -For the captured documentation-ready/code-pending case, the proposed anatomy is: - -```text -Found 1 documentation result. - -Sources: - - site:expressjs.com (hosted documentation) - -Preparing: - - github:expressjs/express@1bb798d9 (indexing, estimated total: 25-61s) - Requested: npm:express@2.3.10 -``` - -#### Per-call concision constraint (user feedback, 2026-10-07) - -Default to exactly one outcome sentence. Sources and Preparing already carry -served scope and pending work; do not add a second sentence paraphrasing them. -In the captured ready-documentation/pending-code case above, there is no extra -`Repository code is still indexing` or `documentation results are usable now` -paragraph. Repetition on every call accumulates unnecessary agent context. - -Add at most one short explanation only when a meaningful limitation or lifecycle -fact is otherwise missing. The notice must add a fact, not restate a label or -teach the output format. Name a cause only when supplied facts establish it; -a preparing refresh does not prove no code was searched. Preserve the deliberate -use-now/conditional-wait guidance when actually offering a wait alongside usable -hits; healthy results without wait advice need no use-now explanation. - -Search backend partialResults remains meaningful, including empty/completed -snapshots. If attributed source/preparation/coverage notes already explain the -missing scope, do not repeat an abstract warning. If partialResults=true has no -such explanation, say `These results do not cover the full request.` without -guessing an indexing cause. Retain this fact in the private availability model; -public JSON stays unchanged. Active work not explained by Preparing can say -`Search is still running.` Known terminal states retain explicit ended/failed -reason sentences and unknown states remain unknown; do not imply completion. - -Grep classification keeps the reviewed distinction, but it now drives prose and -empty outcomes rather than abstract headline qualifiers: - -- Actual gaps: readiness other than CURRENT except the documented unvisited - UNSPECIFIED+RESUMABLE_LIMIT case; non-pagination traversal, errors, skips and - scan issues, or overall NON_RESUMABLE_PARTIAL/FAILED/CURSOR_EXPIRED not explained solely by omitted targets. -- Only retryable omissions: no actual gap, at least one unavailableTarget and - all omissions retryable. NON_RESUMABLE_PARTIAL is also emitted for omission-only pages; the existing Preparing/Omitted rows explain that temporary limit without another traversal warning. -- Non-retryable omissions or actual gaps: existing attributed coverage/reason - notes explain the limit. When no existing note conveys the overall limitation, - use `Some requested content could not be searched.` without inventing a cause. -- CURRENT+RESUMABLE_LIMIT and unvisited UNSPECIFIED+RESUMABLE_LIMIT without - independent errors/skips are ordinary pagination, not failures. Keep unvisited - source qualifiers and the exact cursor; no unnecessary retry. - -Preserve the strict exhaustive predicate. The revised zero-page examples are: - -| Returned page | Outcome | -| --- | --- | -| Exhaustive search/grep | `No results found.` / `No matches found.` | -| Active search or only retryable grep omissions | `No results available yet.` / `No matches available yet.` | -| Empty continuation page | `No results on this page.` / `No matches on this page.` | -| Empty continuation plus retryable grep omissions | `No matches available yet on this page.` | -| Actual missing/failed scope | Plain no-results/no-matches outcome plus the attributed limitation explanation | -| No search snapshot | `No results available yet.` while active, or explicit ended-search explanation otherwise | - -Pagination remains independently available under More results. Partial empty -snapshots must not imply an exhaustive no-result search; use the scope explanation -or the full-request warning above. These copy choices were settled by the user -and are now under implementation review; prior plan reviews covered the -semantics, not this revised wording. - -### Body and source sections - -Order stays outcome/lifecycle, Sources, Preparing, scope warnings/recovery, -then tool-native hits/matches. Do not move actionable per-target remediation -into an unattributed global footer. Preserve source ordering, requested aliases, -zero-hit source disclosure, actual job identity, dates and all coverage facts. - -### Footer - -Use three optional sections, in this fixed order, separated by one blank line: - -1. **Read:** existing concrete example or file/page templates. -2. **More results:** repeat the original request with its exact continuation. -3. **Follow-up:** existing optional polling, omitted-target retry, fresh-search or - query-rewrite advice, with its conditions retained. - -If the existing search action has useResults=true but no returned read action -exists, omit Read and retain `Use these results now.` as the first plain-prose -line in Follow-up, before its conditional wait/fresh-search advice. More results -remains earlier in the footer. Never invent a read locator. - -No section is printed without a real action or meaningful advisory. Header and -labels use identical wording on CLI and MCP; action syntax remains native. - -Search selects the first actual returned read action as an example even on a -healthy completed page. On a healthy page with no wait advice, show the read -command without instructional prose. When a wait is also offered, retain the -short `Use these results now; example read:` lead to prevent wait-first behavior. Grep retains file/page templates, removes -only their leading `#` and prefixes CLI templates with `githits` so they are -consistent command recipes; no template appears for an empty page. Example: - -```text -Read: - Use these results now; example read: - githits read 'https://expressjs.com/llms/resources.txt' --selector 'route' - -More results: - Repeat the original search, adding: - --offset 1 - Results may change while this search is running. - -Follow-up: - If you need updated results, wait (hits and order may change): - githits search-status --wait 80 -``` - -```text -Read: - Pages: githits read --lines $start-$end -- $url - -More results: - Repeat the original grep, adding: - --cursor '' - -Follow-up: - To retry omitted targets, rerun the original query with --wait 80000. -``` - -Examples above are designed layouts using captured facts, not new live renders; -placeholder locators are explicitly illustrative. MCP uses the same labels with -`read target=...`, `offset=1`, `cursor=...`, `search_status search_ref=...` and -`wait_timeout_ms=...`. Search CLI wait remains seconds; grep wait remains ms. -The implementation must preserve the existing exact native read arguments instead -of deriving them from display labels; the grep CLI prefix changes presentation -only, not the command or operands. - -For active/interim search pagination, add under More results: -`Results may change while this search is running.` Repeating search is not -continuing an immutable snapshot. Retained terminal results instead preserve -their existing mutable-evidence warning and fresh-search requirement; an ended -search reference never becomes pollable. More results always says *original -search*, including on search-status output, because its query echo cannot -reconstruct caller filters/targets. hasMore with no nextOffset remains a truthful -`More results are available; repeat the original search.` advisory without -inventing an offset. Never compute it from the visible hit count. - -Prior-HEAD specific-ref guidance stays attached to the search read/follow-up advice. -Keep `If you need current HEAD` when that proof exists, the hits/order warning, -query-rewrite choices and no-poll terminal rules. Known terminal statuses are -DEFERRED/TIMEOUT/FAILED; other status values retain existing `status unknown` -wording rather than claiming a new backend lifecycle contract. Empty results have no Read -section; pagination and recovery are independently optional, not gated by the -lifecycle action union. - -The full cursor cannot become shorter within the existing contract. Keep it -unwrapped and exact, dim action lines on ANSI CLI in both tools, and place each on its own -indented line. Section labels are bold, prose is plain, and all footer action -lines (read, offset/cursor and status commands) are dim. Use a tiny shared -action-line styling function; per-tool renderers pass exact action strings, so -the helper never guesses whether prose is a command. ANSI-free output carries -identical words, operands and order. It will still take space; truncation, -local handles, files, clipboard integration or a new cursor API are out of scope. - -Search alternative summaries use `(+N more)` rather than `+N`. Preserve existing -limits, ordering, version/ref categories and suggested-versus-indexed meaning. -Uncounted `+more` evidence retains its unknown-count meaning; no synthetic count. -Read/list/resolve output and their alternatives do not otherwise change in this -PR. Document this intentional scope boundary so their older footer labels are -not mistaken for accidental drift. - -## Architecture and scope - -Shared MCP presentation naturally owns repeated output copy because CLI and MCP -already call these neutral formatters. Per-tool adapters own counts, lifecycle, -coverage and native actions; core services continue owning data and validation. -A Commander-level helper would duplicate MCP behavior; a core helper would put -presentation in transport. Neither is appropriate. - -Use one small pure `packages/mcp/src/shared/search-grep-output-text.ts` helper for -the fixed Read/More results/Follow-up section skeleton only. Headline sentences -reuse existing terminal prose wrapping and emphasis in each tool formatter; -there is no reason for a separate shared counter/joining abstraction. -Its inputs are optional -read/more/follow-up lines; it knows no backend state, target identity or command -arguments. Treat action lines as verbatim strings; only formatter-authored prose -is wrapped before supplying it. No configurable section registry, formatter DSL, -state machine, service DTO, runtime dependency or public export is needed. - -Tool formatters retain all decisions and use this helper. Footer pagination is -rendered separately from search's existing lifecycle action; there is no reason -to add pagination to that semantic union. Reuse `renderReadTarget`, exact quoting, -existing wrapping/colors and wait policy. Header wrapping must work on both -surfaces; current search's unsplit first line needs the same width treatment as -existing grep prose. Do not rewrap returned source content or action operands. - -Likely production files: `unified-search-text.ts`, `unified-search-status-text.ts` -(only if passing existing result facts needs adjustment), `grep-text.ts` and the -new small helper, plus affected structural assertions in -`scripts/cli-smoke.ts` and `packages/mcp/src/smoke-test.ts`. These validators -currently recognize `Next:` and old grep read labels; migrate only search/grep -assertions, leaving unrelated resolve/list checks intact. The existing -`UnifiedSearchAvailability` private projection gains the supplied partialResults -boolean (false when no snapshot), so empty snapshots can retain partial truth. -That fact naturally belongs in availability, not duplicated in CLI adapters or -inferred from hit count; add a focused projection test. No response/service/request/schema/descriptor changes -are planned. JSON and API field selections stay byte/structurally equivalent. -Shared header/footer placement replaces duplication without reworking result -bodies or source/preparation ownership. Existing mapped errors retain their -contracts; this change concerns successful/retained result text, not auth errors. - -## Phase 1 — consistent and truthful result edges - -- Status: **IN PROGRESS**. -- Expected outcome: the examples above hold for CLI/MCP search/status and grep; - usable results, pending scopes and available actions are immediately clear. -- Assumptions: existing nextOffset/cursor/read/lifecycle facts suffice (verified - above); fixed three-section helper needs no new service data; long cursors - remain unavoidable within the existing contract. -- Unknowns/product decisions: **none** after the user invoked $implement on the - revised design. Fresh implementation review covers the final approved wording. -- Dependencies: reviewed plan, merged source rows and current main baseline. - -Ordered implementation: - -1. Add behavioral fixtures for headers and independently optional footer actions - in existing formatter tests. Add the small shared helper and integrate search - plain outcome sentences and evidence-based limitation explanations. -2. Separate search read/pagination/follow-up rendering while preserving its semantic - action projection; integrate grep's existing actions through the same skeleton. - Preserve exact locators/cursors/wait units and all target recovery/body output. -3. Normalize search's counted alternative suffix. Scan affected structural smoke - assertions and parity tests for old header/footer assumptions; update only - expectations covered by this contract. -4. Update `search-snapshot-presentation.md`, `unified-grep.md`, - `mcp-cli-parity.md` and relevant output examples in `cli-commands.md`/`tools.md`. - Add an independent changes fragment with pending **patch** impact for both - githits and @githits/mcp (text-only behavior). No version/changelog edits. -5. Focused verification, internal review and a fresh Claude implementation review - loop; commit/push and one draft implementation PR. No merge or release. - -Acceptance cases: - -- Ready search code/docs/mixed hits and ready grep multiple occurrences on one - line: counts are correct and not repeated, JSON unchanged, plain sentences and - the same footer section labels; no pipe-separated counters or unexplained flags. -- Ready docs plus pending code, including multiple targets and duplicate aliases: - Sources/Preparing stay truthful; no ambiguous readiness fraction; search and - grep explain pending work through attributed Preparing facts rather than an - unexplained partial label; no extra sentence restates Preparing or the usable - Sources. Grep normal pagination alone never claims incomplete coverage. -- Active interim and completed partial search: explain continuing work and - incomplete request coverage in plain language independently of completed state. PENDING/INDEXING/SEARCHING and - DEFERRED/TIMEOUT/FAILED/unknown remain visible and receive only their valid actions. -- Empty complete, empty active, absent snapshot, zero-hit continuation pages, - withheld scopes, actual coverage issues and expired grep cursor: precise - outcomes, no spurious Read, unchanged scope remediation. -- Search/status nextOffset and hasMore-without-offset, active mutable ordering, - terminal retained pages and grep cursors: More results is independent of Follow-up, - original-request controls retained, no status offset or invented operand. -- Multi-target grep with an unvisited UNSPECIFIED+RESUMABLE_LIMIT scope: - both hit-bearing and zero-hit continuation pages omit false coverage-incomplete - copy, keep the source page qualifier and exact cursor, with no invented retry. -- Grep zero-hit docs continuation with a retryable pending repository, and the - same shape with non-retryable omissions: exact table outcomes, correct - coverage qualifier, retained cursor and attributed omission reasons. -- Active search with hits: explicit `Use these results now` before - `If you need updated results, wait`; no required retry implied for usable hits. - Readless active hits retain use-now as the first Follow-up line, omit Read and - never invent a read target. -- Prior HEAD, current HEAD and ended evidence: read-now before optional wait, - exact specific-ref advice, no pollable ended search. -- Repeated-call concision: ordinary ready and ready-docs/pending-code pages - contain one outcome sentence, no redundant status explanation, and action-only - healthy read guidance. Exceptional notes add otherwise missing facts; conditional - waits with usable results retain the behavioral use-now safeguard. -- 40/80/120-column output, ANSI stripped versus plain output and backend Unicode: - prose wraps, fixed actions/content do not; all footer sections omit cleanly. - -Verification: use bun test for the shared helper and existing -`unified-search-text.test.ts`, `unified-search-status-text.test.ts`, -`unified-search-snapshot-text.test.ts`, `unified-search-presentation.test.ts`, -`grep-text.test.ts`, `grep-text-rendering.test.ts`, `grep-response.test.ts`, -`indexing-estimates.test.ts`, `unified-search-semantic-text.test.ts`, and root -`search-parity.test.ts`/`grep-parity.test.ts`. Run affected smoke-assertion unit -cases, typecheck and both builds. Run `bun run smoke:cli`, `bun run smoke:mcp`, -`bun run smoke:cli:built` and `bun run smoke:mcp:built`; built checks verify -changed structural smoke assertions against packed Node launch paths. Run public -package validation after builds. Broader unit suite is the final integration -check after formatter changes; do not repeat it after wording-only closure. - -For live verification, use authenticated dev only, remove unintended endpoint -and token overrides without displaying credentials, and compare search/grep -on an indexed pinned package and a registry-confirmed unindexed package. -Use `route` search and literal `foo` grep for Express documentation, limit 1, -and the same pinned input within each client; if the version becomes indexed, -record the transition and verify pending cases against a different verified -unindexed version rather than inventing its version number. Use -literal CLI --wait 1 and matching MCP waits, record actual input/version and -per-response states. Use fixtures for terminal/unknown/expired states rather -than waiting for sessions to expire. Inspect captured output manually as well as -structural checks. Agent-facing text behavior also requires targeted -`bun run agent:e2e` search-investigation and grep-mixed-docs workloads selected -from eval/agentic/README.md: -`GITHITS_ENV=dev bun run agent:e2e --agent claude --surface mcp --server local --workload eval/agentic/workloads/unified-search-investigation.md` -and the same command with -`--workload eval/agentic/workloads/grep-mixed-docs.md`. -Inspect actual calls/final answer/isolation/metrics. -Previous Claude runs failed provider login before tool use, which is no quality -result; verify current availability and report the same limitation if it persists. -No descriptor/instruction changes or broad routing eval is implied. - -No new auth, deployment, performance or network risk is introduced. Programmatic -callers retain JSON; users receive revised text-v1 prose. Existing partial -search opt-out, wait defaults, selectors, filters, ordering and page size remain. -Hosted MCP adoption still follows its package release/dependency/deployment path. - -## Review, completion and cleanup - -Plan review: internal technical review followed by Claude Opus 5.5 external -rounds until clean (maximum three); adjudicate findings against the selected scope and verified facts. -Implementation review is a separate fresh loop. Keep the plan through its clean -round, move final facts/examples to permanent implementation docs, then delete -this plan in the final implementation PR commit. No separate cleanup PR. -Remove the selected backlog entry now that this plan owns it, replacing it with -one link while pending; delete that link on completion. The broader -`search-output-ux.md` plan's unrelated per-tool migrations are not absorbed; -this user-selected two-tool follow-up is an explicit narrow cross-tool increment. -One phase means no intermediate merge/reorientation boundary. If verified evidence -requires another phase or a broader design, stop and replan with the user. - -Historical review of the superseded pipe-separated proposal follows. It does -not establish readiness of the current headline revision. - -Internal technical review was **clean** after correcting empty partial-search -provenance, empty resumable grep wording and the known terminal status list. -The private availability correction is the smallest boundary change needed; -public JSON and service contracts stay unchanged. External Claude round 1 accepted direction and found five plan corrections: -read-now/conditional wait wording, omitted-target pagination semantics, action -styling, grep CLI read prefix and missing focused test files. All are accepted -and applied. Round 2 accepted four closures and refined the remaining grep -wording: retryable omissions only now say partial, while actual gaps and -non-retryable omissions say coverage incomplete. The readless advisory has an -explicit Follow-up slot; the read/follow-up wording is corrected. Round 3 found one remaining documented unvisited-scope case: -UNSPECIFIED+RESUMABLE_LIMIT is ordinary pagination, now exempted consistently -in header/omissions-only/zero-page rules and acceptance. The three-round cap is -reached; no fourth external round is dispatched. Coordinator closure verifies -this against implementation documentation and its existing fixture tests. -Final internal technical closure is clean. Existing unvisited-scope contract -proof passed: `bun test packages/mcp/src/shared/grep-text.test.ts -packages/mcp/src/shared/grep-response.test.ts --test-name-pattern 'lists unvisited -scopes beside the matched source|retains an unvisited selected site'` — 2 passed, -0 failed. These tests verify the baseline contract, not the proposed new output. -All findings are corrected; no direction, scope or product question remains. -There was no finding-free external round within the cap; implementation will -receive its own fresh review loop and actual output verification. No production change or planning-only PR created. - -## User-directed headline revision (2026-10-06) - -The previous READY state and pipe-separated examples are superseded by the -feedback above. The underlying reviewed continuation/coverage semantics and -footer design remain available; headline copy and the smaller footer-only helper -are proposed, not reviewed-ready. Do not treat historical review closure as -approval of the revised words. No production code changed. - -The user additionally requires per-call token discipline on 2026-10-07: default -header-only outcome, no paraphrase of Sources/Preparing, and exceptional prose -only for otherwise undisclosed facts. This refines the proposed copy and -acceptance; it is not an output-token reduction claim before implementation. - - -## Implementation evidence (2026-10-07) - -Implemented inline under $implement on merged main bc295b3. Private availability -retains backend partialResults for completed/empty snapshots; no public JSON, -request/query selection, descriptors, auth or other commands changed. The shared -helper owns fixed footer layout only; tool renderers retain state and exact -operands. Grep gap checks reuse the same target predicate while the exhaustive -predicate remains stricter. No new infrastructure or major deferral. - -Focused closure: 297 pass, zero fail, 932 assertions across four renderer/smoke -contract files. Full verification passed 5,624 tests/zero failures/22,404 assertions across235 files; typecheck, both builds, source and built CLI/MCP smokes and packed public-package validation passed. Targeted -Claude agent:e2e search-investigation and grep-mixed-docs executions failed with -`Not logged in` before any tool calls. Both tool traces are empty, final/isolation -artifacts absent and usage unknown; no comprehension/quality claim follows. - -Same fixed fixtures before/after: ready search 89 -> 163 bytes (+74), preparing -search 459 -> 553 (+94), mixed paged grep 6992 -> 6985 (-7). Search adds useful -native read/pagination actions that the baseline omitted. This is output size, -not token count, latency or agent-quality evidence. - -Authenticated dev CLI express@2.3.12 (registry-confirmed) returned one docs hit -and one hosted-doc grep match with Sources, repository Preparing, then Read, -More results and Follow-up. Exact command operands, alias and indexed alternatives -remain. MCP grep express@2.4.0 showed the same hierarchy with native syntax. -Initial MCP search encountered a transient Keychain error; retry succeeded after -that version had indexed, proving healthy code Read+pagination without a wait. -Fresh pending MCP search express2.4.1 and immediate retained status succeeded with identical text, exact read selector/offset1/native status wait70000. Healthy CLI/MCP search returned code with Read+More and no wait. - - -Internal finding closure: direction sound. Overall NON_RESUMABLE_PARTIAL/FAILED -can retain a cursor (parser accepts this valid shape); the old no-cursor-only -warning hid the limitation in that case. Fixed overall warning selection while -retaining exact cursor and omission-only silence. Sibling scan covered all overall -traversal branches, target gap/exhaustive predicates, parser enum/cursor contracts, -zero/hit pages and related documentation. Added both statuses with zero/hit pages; -no service/state change or speculative mechanism. Two stale grep-doc outcome -paragraphs now match current prose. Internal revised-delta closure is clean; 244 tests/zero failures/913 assertions across eight affected files prove the fix. Closure typecheck, both builds and source/built CLI/MCP smoke checks passed. - - -External round 1: direction sound, not clean (one code finding, one doc nit). -F1 accepted: empty cursor pages gated their page qualifier on coverage/omission -classification, so ordinary skipped-file + cursor and terminal-omission + cursor -pages could sound exhaustive. Root invariant is that a returned cursor scopes the -outcome to this page independently of missing coverage; warnings own limitations. -Removed that gate. Closure scan covers all empty grep branches, source exhaustive -qualifiers, target and overall failure warnings, retryable/non-retryable omissions, -normal/unvisited pagination and search/status's corresponding page qualifier. -Tests assert headlines for skipped-file and failed/partial cursor pages, plus a -non-retryable-omission cursor page. F2 accepted: replaced a dangling permanent-doc -review placeholder with the verified closure. Also restored explicit lifecycle -sentence assertions where a migrated test left an unused label. No machinery or -scope expansion. Nine-file closure passes 354 tests, zero failures and 1,353 -assertions. Internal revised-delta closure is clean. Typecheck, both builds, and -source/built CLI/MCP smoke checks pass after the external correction; external -round 2 pending. - -External round 2: direction sound; R1 behavior and closure correct. Low finding -accepted: the overall failed/partial fixture asserted warning/cursor but omitted -the headline assertion claimed above. Root class is evidence that does not prove -a documentation claim. Scanned the new skip/omission/overall fixture assertions -against those claims; added explicit zero/hit headlines for both overall states. -Moved the omission test into the related describe block (accepted test-organization -nit). Production is unchanged. Focused `bun test packages/mcp/src/shared/grep-response.test.ts` -passes 19 tests, zero failures and 118 assertions; scoped Biome and diff checks -pass. Internal full revised-delta closure is clean. Round 3 pending.