From 8cded583afc0e4cd7421d22f8d9008481a11d954 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Tue, 28 Apr 2026 12:24:10 +0300 Subject: [PATCH 1/2] feat(mcp): reduce agent token burn with text-v1 format and code_read cap Real-session traces showed GitHits MCP responses dominating agent context (4M tokens / session), driven by JSON wrapping on every tool plus broad code_read calls (300-600 line windows, occasional unbounded reads). This change adds a compact text-v1 default to search / code_files / code_grep, hard-caps code_read at 150 lines per call on the MCP surface, and rewrites the MCP server instructions block to lead with delegation guidance. Across three test sessions the changes brought total tokens from 4.06M -> 2.06M and code_read max single payload from 22.8 KB -> 6.8 KB. - Add `format: "json" | "text" | "text-v1"` to search, code_files, and code_grep tools. Default flipped to text-v1; programmatic / parity callers opt into JSON. - Add MCP-side cap (MCP_READ_MAX_SPAN = 150) to code_read with a `hint` field describing returned vs. requested ranges. CLI command bypasses the cap so humans can still pipe whole files. - Hint reports the actual returned range (from the response payload) and is suppressed when the cap was a no-op (file fits within cap) or when the returned range reaches end of file. - Expand mcp-instructions CORE_BLOCK trigger criteria to cover comparative cross-OSS questions and "how does X actually work" archaeology. Add gated REFERENCE_FIRST_TIP and a strengthened MULTI_TURN_TIP that leads the package-tools section. - Update per-tool bullets and DESCRIPTIONs to reflect text-v1 defaults instead of JSON-shaped fields. - Document text-v1 format spec, code_read cap, and `hint` field in docs/implementation/tools.md. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude-plugin/marketplace.json | 2 +- .claude-plugin/plugin.json | 2 +- .plugin/plugin.json | 2 +- docs/implementation/tools.md | 90 +++++++- gemini-extension.json | 2 +- package.json | 2 +- plugins/claude/.claude-plugin/plugin.json | 2 +- src/commands/mcp-instructions.test.ts | 31 +++ src/commands/mcp-instructions.ts | 33 ++- src/shared/grep-repo-text.test.ts | 216 +++++++++++++++++ src/shared/grep-repo-text.ts | 269 ++++++++++++++++++++++ src/shared/index.ts | 6 + src/shared/list-files-text.test.ts | 82 +++++++ src/shared/list-files-text.ts | 88 +++++++ src/shared/read-file-request.test.ts | 8 +- src/shared/read-file-request.ts | 4 - src/shared/read-file-response.test.ts | 17 ++ src/shared/read-file-response.ts | 12 + src/shared/unified-search-text.test.ts | 249 ++++++++++++++++++++ src/shared/unified-search-text.ts | 258 +++++++++++++++++++++ src/tools/grep-repo-parity.test.ts | 6 +- src/tools/grep-repo.test.ts | 51 +++- src/tools/grep-repo.ts | 24 +- src/tools/list-files-parity.test.ts | 12 +- src/tools/list-files.test.ts | 66 +++++- src/tools/list-files.ts | 42 +++- src/tools/read-file-parity.test.ts | 13 +- src/tools/read-file.test.ts | 238 +++++++++++++++++++ src/tools/read-file.ts | 166 +++++++++++-- src/tools/search.test.ts | 63 +++++ src/tools/search.ts | 29 ++- 31 files changed, 2008 insertions(+), 77 deletions(-) create mode 100644 src/shared/grep-repo-text.test.ts create mode 100644 src/shared/grep-repo-text.ts create mode 100644 src/shared/list-files-text.test.ts create mode 100644 src/shared/list-files-text.ts create mode 100644 src/shared/unified-search-text.test.ts create mode 100644 src/shared/unified-search-text.ts diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 7a89ae70..e47e07f8 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -6,7 +6,7 @@ }, "metadata": { "description": "GitHits plugins for Claude Code - code examples from global open source", - "version": "0.2.2" + "version": "0.2.3" }, "plugins": [ { diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 9be464a1..06441cc7 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.2", + "version": "0.2.3", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits" diff --git a/.plugin/plugin.json b/.plugin/plugin.json index 9be464a1..06441cc7 100644 --- a/.plugin/plugin.json +++ b/.plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.2", + "version": "0.2.3", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits" diff --git a/docs/implementation/tools.md b/docs/implementation/tools.md index 3be1a2b6..d19c0911 100644 --- a/docs/implementation/tools.md +++ b/docs/implementation/tools.md @@ -8,12 +8,10 @@ The CLI exposes MCP tools that mirror the backend's MCP server. This document ex GitHits has two MCP server implementations: -- **Backend** (`githits-backend`) — Python/FastMCP, runs as a hosted service at `mcp.githits.com` -- **CLI** (`githits-cli`) — TypeScript/MCP SDK, runs locally via `githits mcp start` +- **Backend** (`githits-backend`) — Python/FastMCP, runs as a hosted service at `mcp.githits.com`. Surfaces only the example-search workflow (`get_example`, `search_language`, `feedback`). +- **CLI** (`githits-cli`) — TypeScript/MCP SDK, runs locally via `githits mcp start`. Surfaces the full set, including unified `search`, package intelligence (`pkg_*`), and code navigation (`code_*`). -Both expose the same tools with identical names, parameters, and descriptions. The backend handles auth, analytics, and AI orchestration server-side. The CLI delegates to the REST API, which is a simpler path that still benefits from the backend's processing pipeline. - -> **Tools must stay in sync with the backend.** When the backend adds or modifies a tool, the CLI must be updated to match. The tool names, parameter names, descriptions, and schema constraints should be identical across both implementations. +The two surfaces overlap on the example-search workflow only; for the overlapping tools the parameters and descriptions stay aligned. Tools that exist only on the CLI surface (code navigation, package intelligence, unified `search`) are not constrained by backend parity. ## Current Tools @@ -22,15 +20,15 @@ Both expose the same tools with identical names, parameters, and descriptions. T | `get_example` | `query`, `language?`, `license_mode?` | Search for canonical code examples. Returns markdown-formatted results. If `language` is omitted, the backend infers it from the query. | | `search_language` | `query` | Find supported programming language names before searching. | | `feedback` | `solution_id`, `accepted`, `feedback_text?` | Submit feedback on a search result to improve quality. | -| `search` | `query`, `target?`, `targets?`, `sources?`, `category?`, `kind?`, `path_prefix?`, `file_intent?`, `public_only?`, `name?`, `language?`, `allow_partial_results?`, `limit?`, `offset?`, `wait_timeout_ms?` | Unified indexed dependency/repository discovery search across code, docs, and symbols. Omit `file_intent` to search across all intents; set it only when you want to narrow results, and note that some sources may ignore it and report that in `sourceStatus`. Prefer `sources:["symbol"]` for symbol-shaped unified search. Complete-by-default; set `allow_partial_results: true` to receive available partial hits while indexing continues. | +| `search` | `query`, `target?`, `targets?`, `sources?`, `category?`, `kind?`, `path_prefix?`, `file_intent?`, `public_only?`, `name?`, `language?`, `allow_partial_results?`, `limit?`, `offset?`, `wait_timeout_ms?`, `format?` | Unified indexed dependency/repository discovery search across code, docs, and symbols. Omit `file_intent` to search across all intents; set it only when you want to narrow results, and note that some sources may ignore it and report that in `sourceStatus`. Prefer `sources:["symbol"]` for symbol-shaped unified search. Complete-by-default; set `allow_partial_results: true` to receive available partial hits while indexing continues. `format` defaults to `text-v1` for compact agent output; pass `format: "json"` for the structured envelope. | | `search_status` | `search_ref` | Check progress, fetch partial hits when the original request used `allow_partial_results: true`, or fetch final results for a prior unified search. | | `pkg_info` | `registry`, `package_name` | Package overview: latest version, license, description, repository, downloads, GitHub metadata, install command, and known vulnerabilities. Always returns the latest published version. | | `pkg_vulns` | `registry`, `package_name`, `version?`, `min_severity?`, `include_withdrawn?` | Known vulnerabilities for a package on npm, PyPI, Hex, or Crates. Count summary, per-advisory OSV ID + severity + affected/fix ranges, and upgrade paths. Malware is surfaced in a disjoint bucket. | | `pkg_deps` | `registry`, `package_name`, `version?`, `lifecycle?`, `include_transitive?`, `include_importers?`, `max_depth?` | Direct runtime dependency list (each `{name, version, constraint}` — the backend resolves each constraint to a concrete version) plus, when the backend has them, structured groups for dev / peer / build / optional with registry-specific condition metadata (PyPI extras, Crates features). Optional transitive block with aggregate edge counts, the preprocessed install footprint as `{name, version}`, typed conflicts and circular-dependency cycles; opt into per-package importer provenance with `include_importers`. | | `pkg_changelog` | `registry?`, `package_name?`, `repo_url?`, `from_version?`, `to_version?`, `limit?`, `git_ref?`, `include_bodies?` | Release notes or changelog entries for a package or GitHub repo. Default latest mode returns the ten most recent entries; `from_version` switches to range mode (no count cap). Dual addressing (spec vs repo URL) mutually exclusive. Response always includes `source` (`"releases"` / `"changelog_file"` / `"hexdocs"`), `mode` (`"latest"` / `"range"`), and `entries: { count, items }` with full markdown bodies by default; set `include_bodies: false` for a lean version / date / URL timeline. | -| `code_files` | `target`, `path_prefix?`, `limit?`, `wait_timeout_ms?` | List files in an indexed dependency. Returns `{total, hasMore, files: [{path, name, language, fileType, byteSize}], resolution, indexedVersion}`. Dual addressing via `target.registry + target.package_name` (spec) or `target.repo_url + target.git_ref` (repo). `path_prefix` is a literal directory prefix — NOT a glob (`*.ts` won't match); glob / pattern filtering is an upstream enhancement. Emits an `INDEXING` error envelope when the dependency is being indexed on-demand — retry with a longer `wait_timeout_ms` or pick a version from `details.availableVersions`. | -| `code_read` | `target`, `path`, `start_line?`, `end_line?`, `wait_timeout_ms?` | Read a file from an indexed dependency. Default full file; use `start_line` / `end_line` for a bounded range. Binary files set `isBinary: true` and omit `content` — agents branch on the flag. On `NOT_FOUND` / `FILE_NOT_FOUND` call `code_files` to discover the actual path. | -| `code_grep` | `target`, `pattern`, `path?`, `path_prefix?`, `globs?`, `extensions?`, `pattern_type?`, `case_sensitive?`, `exclude_doc_files?`, `exclude_test_files?`, `context_lines?`, `context_lines_before?`, `context_lines_after?`, `max_matches?`, `max_matches_per_file?`, `cursor?`, `symbol_fields?`, `wait_timeout_ms?` | Deterministic text grep over indexed dependency or repository source. Defaults to literal, ASCII case-insensitive matching across the whole target; non-ASCII letters match case-sensitively. Narrow with `path`, `path_prefix`, `globs`, or `extensions`. `pattern_type: "regex"` uses RE2 syntax; whole-target regexes must include at least one literal substring for index pre-filtering. Returns matches plus pagination and scan counters; `symbol_fields` hydrates enclosing symbol metadata on each match. | +| `code_files` | `target`, `path_prefix?`, `limit?`, `wait_timeout_ms?`, `format?` | List files in an indexed dependency. Returns `{total, hasMore, files: [{path, name, language, fileType, byteSize}], resolution, indexedVersion}` in JSON mode. Dual addressing via `target.registry + target.package_name` (spec) or `target.repo_url + target.git_ref` (repo). `path_prefix` is a literal directory prefix — NOT a glob (`*.ts` won't match); glob / pattern filtering is an upstream enhancement. Emits an `INDEXING` error envelope when the dependency is being indexed on-demand — retry with a longer `wait_timeout_ms` or pick a version from `details.availableVersions`. `format` defaults to `text-v1` (paths-only listing); pass `format: "json"` for the structured envelope. | +| `code_read` | `target`, `path`, `start_line?`, `end_line?`, `wait_timeout_ms?` | Read a file from an indexed dependency. **MCP per-call span cap: 150 lines** — broader requests (or no range) are silently truncated to the first 150 lines from the caller's start, with a `hint` field explaining the cap and the original request. Use `start_line` / `end_line` to pick a focused window — typical 80-150 lines around a known symbol from `search` / `code_grep`. Binary files set `isBinary: true` and omit `content`. On `NOT_FOUND` / `FILE_NOT_FOUND` call `code_files` to discover the actual path. The cap is MCP-only; the CLI command `githits code read` honors arbitrary ranges. | +| `code_grep` | `target`, `pattern`, `path?`, `path_prefix?`, `globs?`, `extensions?`, `pattern_type?`, `case_sensitive?`, `exclude_doc_files?`, `exclude_test_files?`, `context_lines?`, `context_lines_before?`, `context_lines_after?`, `max_matches?`, `max_matches_per_file?`, `cursor?`, `symbol_fields?`, `wait_timeout_ms?`, `format?` | Deterministic text grep over indexed dependency or repository source. Defaults to literal, ASCII case-insensitive matching across the whole target; non-ASCII letters match case-sensitively. Narrow with `path`, `path_prefix`, `globs`, or `extensions`. `pattern_type: "regex"` uses RE2 syntax; whole-target regexes must include at least one literal substring for index pre-filtering. Returns matches plus pagination and scan counters; `symbol_fields` hydrates enclosing symbol metadata on each match. `format` defaults to `text-v1` (matches grouped by file, grep -A/-B notation for context); pass `format: "json"` for the structured envelope. | `search`, `search_status`, `pkg_info`, `pkg_vulns`, `pkg_deps`, `pkg_changelog`, `code_files`, `code_read`, and `code_grep` are only registered when the current token explicitly carries the `code_navigation` feature flag. The package/source service URL defaults to `https://pkgseer.dev` in the default production environment and can be overridden via `GITHITS_CODE_NAV_URL` for local development. @@ -139,14 +137,84 @@ All three code-navigation tools share the same indexing-retry contract. The stat **`FILE_NOT_FOUND` vs `NOT_FOUND`**: `code_read` / `code_grep` can hit "path doesn't resolve" errors when an exact path scope is invalid. The classifier is pre-wired to emit `FILE_NOT_FOUND` when the backend sends `extensions.code: "FILE_NOT_FOUND"`, but today the backend emits generic `NOT_FOUND` for both "package missing" and "path missing". The distinction is filed upstream. CLI terminal output for `code read` / `code grep` emits the hint "Use `code files` to list available paths." on both codes so users have an actionable next step regardless of classification. +**`code_read` span cap (MCP-only)**: real session traces showed agents requesting 300-600 line windows (and occasional unbounded full-file reads) which dominated context cost. The MCP surface caps each `code_read` call at `MCP_READ_MAX_SPAN` (150 lines, defined in `src/tools/read-file.ts`). The cap is enforced *before* the backend call — `deriveBoundedRange` clamps the request, so the service does not transfer bytes that will be discarded. When clamping fires, the envelope carries a `hint` field describing what was returned, what the caller originally requested, and the 80-150-line guidance for a follow-up. Binary files skip the hint. The CLI command `githits code read` does not apply the cap; humans piping whole files to disk continue to work. + +## Text response format (`format: "text-v1"`) + +`search`, `code_files`, and `code_grep` accept a `format` parameter on the MCP surface. The default is `"text-v1"` — a compact line-oriented format that drops JSON scaffolding to stay lean in agent context. Programmatic callers (parity tests, scripts that parse responses) pass `format: "json"` explicitly. `"text"` is accepted as an alias for `"text-v1"` to keep agent prompts terse. + +**Why text-v1 default.** A 10-hit `search` JSON envelope runs 5–7 KB after compaction; the same hits in `text-v1` land around 3–4 KB. The savings come from dropped quoting, dropped key repetition, and dropped fields that an agent does not need at the per-call decision point (highlights byte offsets, repeated locator scaffolding). The token budget belongs to the agent's reasoning, not to JSON structure. + +**Format stability.** The text format is a public contract, locked with snapshot-style tests (`src/shared/unified-search-text.test.ts`, `src/shared/list-files-text.test.ts`). The `text-v1` version tag exists so we can evolve the format without silently breaking downstream parsers — future incompatible changes ship as `text-v2`. + +**ASCII-only.** Separators are ` | `; ellipsis is `...`; no box-drawing or Latin-1 punctuation. Tokenizer behavior for multi-byte UTF-8 varies across BPE variants, and the format runs into Claude, Codex CLI, OpenCode, Cline, Cursor, etc. — ASCII keeps it predictable. + +**Hit anatomy** (`search` text-v1): + +``` +search | hits | query="..." +[blank] +[1] + + + + +[blank] +[2] ... +[blank] +More hits available. Pass offset=N for the next page or limit=N to widen. +``` + +`` compacts to `code` / `symbol` / `docs` / `repo-docs`. `` is `path:start-end` (optionally followed by ` | qualifiedPath | kind`) for code/symbol hits; `pageId: ` for documentation pages; or `sourceUrl` as a last resort. The locator line is copy-pasteable as `code_read` arguments. + +**Listing anatomy** (`code_files` text-v1): + +``` +code_files | [+] paths | [path_prefix="..."] [limit=N] +[blank] + + +... +[blank] +More files available. Pass limit=N to widen or refine path_prefix. +``` + +`` is `:@` for spec addressing or `@` for repo addressing. Filter echoes appear in the header only when the caller supplied them explicitly (defaults never echo). + +**Grep anatomy** (`code_grep` text-v1): + +``` +code_grep | matches in files | pattern="..." [regex,case-sensitive] [filter echo] +[blank] + () + 142: matching line content + 287: another match +[blank] + () + 140- context-before line + 141- context-before line + 142: matching line + 143- context-after line +[blank] +[Truncated: limit. Pass narrower path/path_prefix/globs or increase max_matches.] +[More matches available. Pass cursor= for the next page.] +``` + +Standard grep -A/-B notation: `:` separator on match lines, `-` on context lines. Non-adjacent blocks within the same file are separated by `--`. The `()` after the file path is the per-file match count; the header sums across files. Header flags (`regex`, `case-sensitive`) appear only when the request used them. Filter echoes (`path_prefix=`, `exts=`, `max_matches=`, etc.) appear only when the caller supplied them. Match-line offsets, file content hashes, file intent, and symbol metadata are dropped in text mode — agents that need them can request `format: "json"`. + +**Errors in text mode.** `search` errors render as text in `text-v1` mode: `search | ERROR | code= [| retryable]\n` followed by an indented `details:` block when present. `code_files` and `code_grep` keep errors JSON-formatted in either mode for now — revisit if agent feedback warrants. + ## Server instructions The MCP server advertises a short, cross-tool orientation via the protocol's server-level `instructions` field. This is distinct from per-tool `description` text: instructions cover rationale, workflow glue, and decisions that span multiple tools, while per-tool descriptions remain the source of truth for arguments, output shape, and tool-specific constraints. `src/commands/mcp-instructions.ts` owns two sections: -- **Core block** — always loaded. Introduces GitHits and the `get_example` / `search_language` / `feedback` workflow. -- **Package-tools block** — appended when `isPackageToolsCapabilityOpen(deps)` is true. Contains a preamble plus one bullet per package tool whose backing service is actually wired, so half-open service configurations never advertise a tool that was not registered. +- **Core block** — always loaded. Introduces GitHits, expands trigger criteria to include comparative cross-OSS questions and "how does X actually implement this" archaeology, and walks through the `get_example` / `search_language` / `feedback` workflow. +- **Package-tools block** — appended when `isPackageToolsCapabilityOpen(deps)` is true. Contains a preamble plus one bullet per package tool whose backing service is actually wired (so half-open service configurations never advertise a tool that was not registered), plus three cross-tool tips emitted only when code navigation is wired: + - **Reference-first, content-second**: locate symbols and lines first, then read narrowly with `code_read` using `start_line` / `end_line` windows around the match. + - **Multi-turn discovery**: anticipate 3+ calls? Delegate to a sub-task / sub-agent and ask for a compact synthesis rather than pulling raw `code_read` / `code_files` output into the main conversation. + - **Tool-selection tip**: contrasts `get_example` vs unified `search` vs `code_grep`/`code_read`. `isPackageToolsCapabilityOpen` is the single source of truth for whether package tools should surface in the current session. Both `getMcpToolDefinitions` and `buildMcpInstructions` import it so tool registration and the instruction text cannot drift. diff --git a/gemini-extension.json b/gemini-extension.json index 6d91c623..3646628a 100644 --- a/gemini-extension.json +++ b/gemini-extension.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.2", + "version": "0.2.3", "description": "Code examples from global open source for developers and AI assistants.", "mcpServers": { "githits": { diff --git a/package.json b/package.json index a8912b4c..62ee0996 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "githits", "description": "CLI companion for GitHits - code examples from global open source for developers and AI assistants", - "version": "0.2.2", + "version": "0.2.3", "type": "module", "files": [ "dist", diff --git a/plugins/claude/.claude-plugin/plugin.json b/plugins/claude/.claude-plugin/plugin.json index 9be464a1..06441cc7 100644 --- a/plugins/claude/.claude-plugin/plugin.json +++ b/plugins/claude/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "githits", - "version": "0.2.2", + "version": "0.2.3", "description": "Code examples from global open source for developers and AI assistants", "author": { "name": "GitHits" diff --git a/src/commands/mcp-instructions.test.ts b/src/commands/mcp-instructions.test.ts index da307635..cfc2229a 100644 --- a/src/commands/mcp-instructions.test.ts +++ b/src/commands/mcp-instructions.test.ts @@ -147,6 +147,37 @@ describe("buildMcpInstructions", () => { expect(instructions).toContain("`search_status`"); expect(instructions).toContain("allow_partial_results"); expect(instructions).toContain("partial hits"); + // Reference-first and multi-turn strategy tips appear when + // code-navigation is wired so agents do not pull raw source into + // the main conversation by default. + expect(instructions).toContain("reference-first"); + expect(instructions).toContain("Delegate multi-call work to a sub-agent"); + expect(instructions).toContain("inherently multi-call"); + // Delegation framing leads the gated section so it lands before + // the per-tool bullets. + const multiTurnIdx = instructions.indexOf( + "Delegate multi-call work to a sub-agent", + ); + const firstBulletIdx = instructions.indexOf("- `"); + expect(multiTurnIdx).toBeGreaterThan(0); + expect(firstBulletIdx).toBeGreaterThan(multiTurnIdx); + }); + + it("expands core trigger criteria to cover comparative cross-OSS questions", () => { + const deps = createTestDeps(); + const instructions = buildMcpInstructions(deps); + expect(instructions).toContain("comparative across OSS projects"); + expect(instructions).toContain("how a real codebase implements"); + }); + + it("does not surface the strategy tips when code-navigation is not wired", () => { + const deps = createTestDeps({ + codeNavigationCapability: "enabled", + packageIntelligenceService: createMockPackageIntelligenceService(), + }); + const instructions = buildMcpInstructions(deps); + expect(instructions).not.toContain("reference-first"); + expect(instructions).not.toContain("Delegate multi-call work"); }); it("keeps the core block first when both sections are present", () => { diff --git a/src/commands/mcp-instructions.ts b/src/commands/mcp-instructions.ts index 5377034f..e4bda816 100644 --- a/src/commands/mcp-instructions.ts +++ b/src/commands/mcp-instructions.ts @@ -15,7 +15,7 @@ import type { Dependencies } from "../container.js"; * current session. */ -const CORE_BLOCK = `GitHits surfaces verified, canonical code examples from global open source. Use it when you're stuck, the user is frustrated by repeated failed attempts, you need up-to-date API usage, or the user mentions GitHits. +const CORE_BLOCK = `GitHits surfaces verified, canonical code examples from global open source. Use it when you're stuck, the user is frustrated by repeated failed attempts, you need up-to-date API usage, the question is comparative across OSS projects (e.g. "how does X vs Y handle Z"), the answer requires reading how a real codebase implements a feature, or the user mentions GitHits. Workflow: call \`get_example\` with one focused question, optionally passing \`language\` when the desired language is known; call \`search_language\` first only if you need to force a language and the exact name is uncertain. Send \`feedback\` on the returned solution_id so quality improves. Each search addresses a single issue; reuse context from prior results before re-searching.`; @@ -42,22 +42,28 @@ const PKG_CHANGELOG_BULLET = "- `pkg_changelog` — release notes for a package or GitHub repo, newest-first. Default latest mode returns the 10 most recent entries with full markdown bodies; `from_version` switches to range mode between two versions. Addressable via `registry` + `package_name` or `repo_url`. Set `include_bodies: false` for a version / date / URL timeline when bodies aren't needed."; const SEARCH_BULLET = - "- `search` — unified search across indexed dependency code, docs, and explicit symbols. Structured fields are the primary UX; omit `sources` for AUTO. Returns only trustworthy complete results by default; opt into partial hits with `allow_partial_results: true`. If indexing is still in progress, the response carries a `searchRef`."; + '- `search` — unified search across indexed dependency code, docs, and explicit symbols. Structured fields are the primary UX; omit `sources` for AUTO. Default response is a compact text listing (`text-v1`); pass `format: "json"` for the structured envelope with full locator fields, highlights, and source status. Returns only trustworthy complete results by default; opt into partial hits with `allow_partial_results: true`. If indexing is still in progress, the response carries a `searchRef`.'; const SEARCH_STATUS_BULLET = "- `search_status` — follow up a prior unified search by `searchRef`. Use it after `search` returns incomplete state to check progress, fetch partial hits when the original request used `allow_partial_results: true`, or fetch final results."; const CODE_FILES_BULLET = - "- `code_files` — discover what files a dependency ships. Use `path_prefix` to scope to a subdirectory; the response includes each file's language, type, and byte size. Returned `path` values feed directly into `code_read` and help scope `code_grep`."; + '- `code_files` — discover what files a dependency ships. Use `path_prefix` to scope to a subdirectory. Default response is a paths-only listing (`text-v1`); pass `format: "json"` for the full envelope with each file\'s language, type, and byte size. Returned paths feed directly into `code_read` and help scope `code_grep`.'; const CODE_READ_BULLET = - "- `code_read` — fetch a file's contents from a dependency. Pass the same `path` emitted by `code_files`. Default returns the full file; pass `start_line` / `end_line` for a bounded range. Binary files set `isBinary: true` and omit `content` — branch on the flag. A `FILE_NOT_FOUND` (or `NOT_FOUND`) response is the signal to call `code_files` for the actual path."; + "- `code_read` — fetch a file's contents from a dependency. Pass the same `path` emitted by `code_files`. **MCP cap: 150 lines per call** — broader requests (or no range) silently truncate to the first 150 lines from your start, with a `hint` describing what was returned vs. requested. Pick a focused window from a `search` / `code_grep` match. Binary files set `isBinary: true` and omit `content`. A `FILE_NOT_FOUND` (or `NOT_FOUND`) response is the signal to call `code_files` for the actual path."; const CODE_GREP_BULLET = - "- `code_grep` — deterministic text grep over indexed source files. Use it when you know the exact text or regex to match; use `search` for discovery. Whole-target grep is the default; narrow with `path`, `path_prefix`, `globs`, or `extensions`. Returned `matches[].filePath` feeds directly into `code_read`."; + '- `code_grep` — deterministic text grep over indexed source files. Use it when you know the exact text or regex to match; use `search` for discovery. Whole-target grep is the default; narrow with `path`, `path_prefix`, `globs`, or `extensions`. Default response groups matches by file with line numbers (`text-v1`); pass `format: "json"` for the `matches[]` array with byte offsets and symbol metadata. Each match\'s `path:line` chains directly into `code_read`.'; const SEARCH_VS_SYMBOLS_TIP = - 'Prefer `get_example` for canonical example retrieval; prefer unified `search` for indexed dependency and repository discovery; use `sources:["symbol"]` when you want symbol-shaped results. Use `code_grep` for deterministic text matching and `code_read` for full-file inspection.'; + 'Prefer `get_example` for canonical example retrieval; prefer unified `search` for indexed dependency and repository discovery; use `sources:["symbol"]` when you want symbol-shaped results. Use `code_grep` for deterministic text matching and `code_read` for focused-window inspection of a known file.'; + +const REFERENCE_FIRST_TIP = + "Strategy — reference-first, content-second. Locate symbols and lines with `search` / `code_grep` first, then read only the lines you actually need with `code_read` using explicit `start_line` / `end_line` windows around the match (typically 80-150 lines). The MCP `code_read` surface caps each call at 150 lines; bigger requests are silently truncated. Each turn — including retries to widen or re-narrow — costs context, so pick a focused window the first time rather than starting wide and trimming."; + +const MULTI_TURN_TIP = + '**Delegate multi-call work to a sub-agent.** Code-navigation tools (`search`, `code_grep`, `code_read`, `code_files`) are inherently multi-call — answering even simple questions usually takes 3-10 tool calls, and reading raw source into the main conversation compounds quickly. The default approach for any cross-project comparison, codebase mapping, pattern survey, or "how does X actually work" investigation is to spawn a sub-task / sub-agent that does the digging and returns only a compact synthesis. Do the work in the main conversation only when the result genuinely belongs there (e.g., the user asked for a specific snippet to paste into their own code). When in doubt, delegate.'; /** * Whether the MCP session should register and describe package tools. @@ -116,10 +122,19 @@ export function buildMcpInstructions(deps: Dependencies): string { return sections.join("\n\n"); } - const parts = [PACKAGE_TOOLS_PREAMBLE, bullets.join("\n")]; - // The decision tip contrasts `get_example` with unified `search`, so - // it is only meaningful when unified search is actually registered. + const parts = [PACKAGE_TOOLS_PREAMBLE]; + // The multi-turn tip leads when code-navigation is wired — it is + // the highest-leverage decision the agent makes (delegate vs. + // run inline) and reading it before the per-tool bullets means + // the framing arrives before any specific tool fires. The + // reference-first / decision tips follow the bullets where they + // act as workflow guidance for the chosen tool. + if (deps.codeNavigationService) { + parts.push(MULTI_TURN_TIP); + } + parts.push(bullets.join("\n")); if (deps.codeNavigationService) { + parts.push(REFERENCE_FIRST_TIP); parts.push(SEARCH_VS_SYMBOLS_TIP); } diff --git a/src/shared/grep-repo-text.test.ts b/src/shared/grep-repo-text.test.ts new file mode 100644 index 00000000..636088d3 --- /dev/null +++ b/src/shared/grep-repo-text.test.ts @@ -0,0 +1,216 @@ +import { describe, expect, it } from "bun:test"; +import type { + LeanGrepRepoEnvelope, + LeanGrepRepoMatch, +} from "./grep-repo-response.js"; +import { renderGrepRepoText } from "./grep-repo-text.js"; + +function envelope( + overrides: Partial = {}, +): LeanGrepRepoEnvelope { + return { + pattern: "applyEdit", + matches: [], + hasMore: false, + filesScanned: 120, + filesInScope: 120, + totalMatches: 0, + uniqueFilesMatched: 0, + ...overrides, + }; +} + +function match(overrides: Partial = {}): LeanGrepRepoMatch { + return { + filePath: "src/diff/foo.ts", + line: 142, + matchStartByte: 16, + matchEndByte: 25, + lineContent: "export function applyEdit(input: string): string {", + ...overrides, + }; +} + +describe("renderGrepRepoText", () => { + it("renders zero-match header and empty body", () => { + const text = renderGrepRepoText(envelope()); + expect(text).toContain("code_grep | 0 matches in 0 files"); + expect(text).toContain('pattern="applyEdit"'); + expect(text).toContain("No matches."); + }); + + it("renders single-file matches grouped under the file with line gutter", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 2, + uniqueFilesMatched: 1, + matches: [ + match({ line: 142 }), + match({ + line: 287, + lineContent: " const result = applyEdit(input, hunk);", + }), + ], + }), + ); + expect(text).toContain("src/diff/foo.ts (2)"); + expect(text).toContain( + " 142: export function applyEdit(input: string): string {", + ); + expect(text).toContain(" 287: const result = applyEdit(input, hunk);"); + }); + + it("renders matches across multiple files with blank-line separators", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 2, + uniqueFilesMatched: 2, + matches: [ + match({ filePath: "src/a.ts", line: 10, lineContent: "a-line" }), + match({ filePath: "src/b.ts", line: 20, lineContent: "b-line" }), + ], + }), + ); + expect(text).toContain("src/a.ts (1)"); + expect(text).toContain("src/b.ts (1)"); + const aIdx = text.indexOf("src/a.ts"); + const bIdx = text.indexOf("src/b.ts"); + expect(aIdx).toBeLessThan(bIdx); + // Two file blocks separated by a blank line. + const between = text.slice(aIdx, bIdx); + expect(between).toContain("\n\n"); + }); + + it("renders context lines with grep -A/-B separators", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 1, + uniqueFilesMatched: 1, + matches: [ + match({ + line: 142, + contextBefore: [ + "// Apply a unified diff patch", + "// Returns the new content", + ], + contextAfter: [' const lines = file.split("\\n");'], + }), + ], + }), + ); + expect(text).toContain(" 140- // Apply a unified diff patch"); + expect(text).toContain(" 141- // Returns the new content"); + expect(text).toContain( + " 142: export function applyEdit(input: string): string {", + ); + expect(text).toContain(' 143- const lines = file.split("\\n");'); + }); + + it("inserts a separator between non-adjacent blocks in the same file", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 2, + uniqueFilesMatched: 1, + matches: [ + match({ + line: 50, + contextBefore: ["before-50"], + contextAfter: ["after-50"], + }), + match({ + line: 200, + contextBefore: ["before-200"], + contextAfter: ["after-200"], + }), + ], + }), + ); + expect(text).toContain(" --"); + }); + + it("renders truncation notice when truncatedReason is set", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 50, + uniqueFilesMatched: 7, + truncatedReason: "limit", + hasMore: true, + matches: [match()], + }), + ); + expect(text).toContain("Truncated: limit."); + expect(text).toContain("max_matches"); + }); + + it("renders next-cursor note when hasMore", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 50, + uniqueFilesMatched: 7, + hasMore: true, + nextCursor: "ABC123", + matches: [match()], + }), + ); + expect(text).toContain("More matches available. Pass cursor=ABC123"); + }); + + it("renders pattern-type and case-sensitive flags in header when set", () => { + const text = renderGrepRepoText( + envelope({ + patternType: "regex", + caseSensitive: true, + totalMatches: 1, + uniqueFilesMatched: 1, + matches: [match()], + }), + ); + expect(text).toContain("regex,case-sensitive"); + }); + + it("echoes filter inputs in header when set", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 1, + uniqueFilesMatched: 1, + matches: [match()], + filter: { + pathPrefix: "src/integrations", + extensions: ["ts", "tsx"], + maxMatches: 30, + }, + }), + ); + expect(text).toContain('path_prefix="src/integrations"'); + expect(text).toContain("exts=ts,tsx"); + expect(text).toContain("max_matches=30"); + }); + + it("uses ASCII separators only", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 1, + uniqueFilesMatched: 1, + truncatedReason: "limit", + hasMore: true, + nextCursor: "X", + matches: [match()], + }), + ); + expect(text).not.toMatch(/[·…—–]/); + }); + + it("notes binary/oversized skips when present", () => { + const text = renderGrepRepoText( + envelope({ + totalMatches: 1, + uniqueFilesMatched: 1, + binaryFilesSkipped: 4, + filesTooLargeSkipped: 2, + matches: [match()], + }), + ); + expect(text).toContain("4 binary file(s) skipped"); + expect(text).toContain("2 oversized file(s) skipped"); + }); +}); diff --git a/src/shared/grep-repo-text.ts b/src/shared/grep-repo-text.ts new file mode 100644 index 00000000..5735484d --- /dev/null +++ b/src/shared/grep-repo-text.ts @@ -0,0 +1,269 @@ +/** + * Line-oriented text renderer for `code_grep` MCP responses. + * + * Designed for agent context efficiency: matches grouped by file, + * `: ` for matches and `- ` for context + * (standard grep -A/-B convention), no per-match scaffolding bytes. + * Programmatic / parity callers stay on the JSON envelope by passing + * `format: "json"`. + * + * ASCII-only output. Format is a public contract — locked with + * snapshot-style tests in `grep-repo-text.test.ts`. + */ + +import type { + LeanGrepRepoEnvelope, + LeanGrepRepoMatch, +} from "./grep-repo-response.js"; + +const SEP = " | "; + +interface RenderLine { + lineNumber: number; + content: string; + isMatch: boolean; +} + +interface RenderBlock { + filePath: string; + lines: RenderLine[]; +} + +export function renderGrepRepoText(envelope: LeanGrepRepoEnvelope): string { + const lines: string[] = []; + lines.push(buildHeader(envelope)); + lines.push(""); + + if (envelope.matches.length === 0) { + lines.push("No matches."); + const trailer = buildTrailer(envelope); + if (trailer.length > 0) { + lines.push(""); + for (const t of trailer) lines.push(t); + } + return lines.join("\n"); + } + + const blocks = buildRenderBlocks(envelope.matches); + const blocksByFile = groupBlocksByFile(blocks); + const useContext = blocksHaveContext(blocks); + const matchCountsByFile = countMatchesByFile(envelope.matches); + + let firstFile = true; + for (const [filePath, fileBlocks] of blocksByFile) { + if (!firstFile) lines.push(""); + firstFile = false; + const matchCount = matchCountsByFile.get(filePath) ?? 0; + lines.push(`${filePath} (${matchCount})`); + fileBlocks.forEach((block, idx) => { + if (useContext && idx > 0) lines.push(" --"); + const gutterWidth = widestLineNumberInBlock(block); + for (const ln of block.lines) { + lines.push(renderLine(ln, gutterWidth, useContext)); + } + }); + } + + const trailer = buildTrailer(envelope); + if (trailer.length > 0) { + lines.push(""); + for (const t of trailer) lines.push(t); + } + + return lines.join("\n"); +} + +function buildHeader(envelope: LeanGrepRepoEnvelope): string { + const parts = [ + `code_grep${SEP}${envelope.totalMatches} match${ + envelope.totalMatches === 1 ? "" : "es" + } in ${envelope.uniqueFilesMatched} file${ + envelope.uniqueFilesMatched === 1 ? "" : "s" + }`, + ]; + parts.push(`pattern=${quote(envelope.pattern)}`); + + const flags: string[] = []; + if (envelope.patternType === "regex") flags.push("regex"); + if (envelope.caseSensitive) flags.push("case-sensitive"); + if (flags.length > 0) parts.push(flags.join(",")); + + const filterEcho = buildFilterEcho(envelope); + if (filterEcho) parts.push(filterEcho); + + return parts.join(SEP); +} + +function buildFilterEcho(envelope: LeanGrepRepoEnvelope): string { + const filter = envelope.filter; + if (!filter) return ""; + const parts: string[] = []; + if (filter.path) parts.push(`path=${quote(filter.path)}`); + if (filter.pathPrefix) parts.push(`path_prefix=${quote(filter.pathPrefix)}`); + if (filter.globs?.length) parts.push(`globs=${filter.globs.join(",")}`); + if (filter.extensions?.length) { + parts.push(`exts=${filter.extensions.join(",")}`); + } + if (typeof filter.maxMatches === "number") { + parts.push(`max_matches=${filter.maxMatches}`); + } + if (typeof filter.maxMatchesPerFile === "number") { + parts.push(`max_matches_per_file=${filter.maxMatchesPerFile}`); + } + return parts.join(" "); +} + +function buildTrailer(envelope: LeanGrepRepoEnvelope): string[] { + const lines: string[] = []; + + if (envelope.truncatedReason) { + lines.push( + `Truncated: ${envelope.truncatedReason}. Pass narrower path/path_prefix/globs or increase max_matches.`, + ); + } + + if (envelope.hasMore && envelope.nextCursor) { + lines.push( + `More matches available. Pass cursor=${envelope.nextCursor} for the next page.`, + ); + } else if (envelope.hasMore) { + lines.push("More matches available."); + } + + // Skip-count notes are useful when something was excluded silently. + const skipNotes: string[] = []; + if (envelope.binaryFilesSkipped) { + skipNotes.push(`${envelope.binaryFilesSkipped} binary file(s) skipped`); + } + if (envelope.filesTooLargeSkipped) { + skipNotes.push( + `${envelope.filesTooLargeSkipped} oversized file(s) skipped`, + ); + } + if (skipNotes.length > 0) { + lines.push(`Note: ${skipNotes.join(", ")}.`); + } + + return lines; +} + +function buildRenderBlocks(matches: LeanGrepRepoMatch[]): RenderBlock[] { + if (matches.length === 0) return []; + + const linesByFile = new Map>(); + for (const match of matches) { + let lineMap = linesByFile.get(match.filePath); + if (!lineMap) { + lineMap = new Map(); + linesByFile.set(match.filePath, lineMap); + } + + const before = match.contextBefore ?? []; + const beforeStart = match.line - before.length; + for (let i = 0; i < before.length; i += 1) { + const lineNumber = beforeStart + i; + if (!lineMap.has(lineNumber)) { + lineMap.set(lineNumber, { + lineNumber, + content: before[i] ?? "", + isMatch: false, + }); + } + } + + // Match line: overwrite a previous context line at the same number + // with a match (matches win over context). + lineMap.set(match.line, { + lineNumber: match.line, + content: match.lineContent, + isMatch: true, + }); + + const after = match.contextAfter ?? []; + for (let i = 0; i < after.length; i += 1) { + const lineNumber = match.line + i + 1; + if (!lineMap.has(lineNumber)) { + lineMap.set(lineNumber, { + lineNumber, + content: after[i] ?? "", + isMatch: false, + }); + } + } + } + + const blocks: RenderBlock[] = []; + for (const [filePath, lineMap] of linesByFile) { + const sorted = [...lineMap.values()].sort( + (a, b) => a.lineNumber - b.lineNumber, + ); + let current: RenderLine[] = []; + for (const line of sorted) { + const previous = current[current.length - 1]; + if (!previous || line.lineNumber === previous.lineNumber + 1) { + current.push(line); + continue; + } + blocks.push({ filePath, lines: current }); + current = [line]; + } + if (current.length > 0) { + blocks.push({ filePath, lines: current }); + } + } + return blocks; +} + +function groupBlocksByFile(blocks: RenderBlock[]): Map { + const map = new Map(); + for (const block of blocks) { + const list = map.get(block.filePath) ?? []; + list.push(block); + map.set(block.filePath, list); + } + return map; +} + +function blocksHaveContext(blocks: RenderBlock[]): boolean { + for (const block of blocks) { + for (const line of block.lines) { + if (!line.isMatch) return true; + } + } + return false; +} + +function widestLineNumberInBlock(block: RenderBlock): number { + let max = 0; + for (const line of block.lines) { + const len = String(line.lineNumber).length; + if (len > max) max = len; + } + return max; +} + +function countMatchesByFile(matches: LeanGrepRepoMatch[]): Map { + const counts = new Map(); + for (const match of matches) { + counts.set(match.filePath, (counts.get(match.filePath) ?? 0) + 1); + } + return counts; +} + +function renderLine( + line: RenderLine, + gutterWidth: number, + useContext: boolean, +): string { + const gutter = String(line.lineNumber).padStart(gutterWidth, " "); + // Standard grep -A/-B notation: `:` separator on match lines, + // `-` on context lines. When there is no context anywhere in the + // response, every line is a match — we still keep the colon so + // the format is uniform. + const sep = !useContext || line.isMatch ? ":" : "-"; + return ` ${gutter}${sep} ${line.content}`; +} + +function quote(value: string): string { + return value.includes('"') ? `'${value}'` : `"${value}"`; +} diff --git a/src/shared/index.ts b/src/shared/index.ts index a4327a4d..e4a2599b 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -53,10 +53,12 @@ export { type LeanGrepRepoFilter, type LeanGrepRepoMatch, } from "./grep-repo-response.js"; +export { renderGrepRepoText } from "./grep-repo-text.js"; export { filterLanguages, type LanguageMatch, } from "./language-filter.js"; +export { renderListFilesText } from "./list-files-text.js"; export { buildListPackageDocsParams, type ListPackageDocsRequestBuildResult, @@ -185,3 +187,7 @@ export { type UnifiedSearchStatusResultPayload, } from "./unified-search-response.js"; export { parseUnifiedSearchTargetSpec } from "./unified-search-target.js"; +export { + renderUnifiedSearchError, + renderUnifiedSearchSuccess, +} from "./unified-search-text.js"; diff --git a/src/shared/list-files-text.test.ts b/src/shared/list-files-text.test.ts new file mode 100644 index 00000000..70c9b4ad --- /dev/null +++ b/src/shared/list-files-text.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "bun:test"; +import type { LeanListFilesEnvelope } from "./list-files-response.js"; +import { renderListFilesText } from "./list-files-text.js"; + +function envelope( + overrides: Partial = {}, +): LeanListFilesEnvelope { + return { + registry: "npm", + name: "express", + indexedVersion: "v5.2.1", + total: 2, + hasMore: false, + files: [ + { path: "src/index.js", language: "javascript", fileType: "SOURCE" }, + { path: "src/lib/app.js", language: "javascript", fileType: "SOURCE" }, + ], + ...overrides, + }; +} + +describe("renderListFilesText", () => { + it("renders a paths-only listing with version-tagged identity", () => { + const text = renderListFilesText(envelope()); + expect(text).toContain("code_files | 2 paths | npm:express@v5.2.1"); + expect(text).toContain("src/index.js"); + expect(text).toContain("src/lib/app.js"); + // No trailing metadata in default mode. + expect(text).not.toContain("javascript"); + expect(text).not.toContain("SOURCE"); + }); + + it("uses repo addressing when no registry is provided", () => { + const text = renderListFilesText( + envelope({ + registry: undefined, + name: undefined, + indexedVersion: undefined, + repoUrl: "https://github.com/cline/cline", + gitRef: "v3.4.2", + }), + ); + expect(text).toContain( + "code_files | 2 paths | https://github.com/cline/cline@v3.4.2", + ); + }); + + it("emits a truncation hint with N+ count when hasMore", () => { + const text = renderListFilesText(envelope({ hasMore: true, total: 2 })); + expect(text).toContain("code_files | 2+ paths"); + expect(text).toContain("More files available."); + }); + + it("echoes explicit filter inputs in the header", () => { + const text = renderListFilesText( + envelope({ filter: { pathPrefix: "src/lib", limit: 50 } }), + ); + expect(text).toContain('path_prefix="src/lib" limit=50'); + }); + + it("renders the empty-result hint when no files match", () => { + const text = renderListFilesText( + envelope({ + files: [], + total: 0, + hint: "No files match this path prefix.", + }), + ); + expect(text).toContain("code_files | 0 paths"); + expect(text).toContain("No files match this path prefix."); + }); + + it("uses ASCII separators throughout", () => { + const text = renderListFilesText( + envelope({ + hasMore: true, + filter: { pathPrefix: "src/", limit: 50 }, + }), + ); + expect(text).not.toMatch(/[·…—–]/); + }); +}); diff --git a/src/shared/list-files-text.ts b/src/shared/list-files-text.ts new file mode 100644 index 00000000..450a2436 --- /dev/null +++ b/src/shared/list-files-text.ts @@ -0,0 +1,88 @@ +/** + * Line-oriented text renderer for `code_files` MCP responses. + * + * Paths-only listing (one file per line) — the most compact useful + * shape for an agent that will follow up with `code_read`. This is + * the tool's default response format; programmatic / parity callers + * opt into the structured JSON envelope via `format: "json"`. + * + * ASCII-only output. Format is a public contract — locked with + * snapshot-style tests in `list-files-text.test.ts`. + */ + +import type { LeanListFilesEnvelope } from "./list-files-response.js"; + +const SEP = " | "; + +export function renderListFilesText(envelope: LeanListFilesEnvelope): string { + const lines: string[] = []; + lines.push(buildHeader(envelope)); + lines.push(""); + + if (envelope.files.length === 0) { + lines.push(envelope.hint ?? "No files match the requested filter."); + return lines.join("\n"); + } + + for (const entry of envelope.files) { + lines.push(entry.path); + } + + if (envelope.hasMore) { + lines.push(""); + lines.push( + "More files available. Pass limit=N to widen or refine path_prefix.", + ); + } + + if (envelope.hint) { + lines.push(""); + lines.push(envelope.hint); + } + + return lines.join("\n"); +} + +function buildHeader(envelope: LeanListFilesEnvelope): string { + const identity = buildIdentity(envelope); + const countValue = envelope.hasMore + ? `${envelope.files.length}+` + : String(envelope.total); + const parts = [ + `code_files${SEP}${countValue} path${countValue === "1" ? "" : "s"}`, + ]; + if (identity) parts.push(identity); + const filter = buildFilterEcho(envelope); + if (filter) parts.push(filter); + return parts.join(SEP); +} + +function buildIdentity(envelope: LeanListFilesEnvelope): string { + if (envelope.registry && envelope.name) { + const version = envelope.indexedVersion ?? envelope.resolution?.resolvedRef; + return version + ? `${envelope.registry}:${envelope.name}@${version}` + : `${envelope.registry}:${envelope.name}`; + } + if (envelope.repoUrl) { + return envelope.gitRef + ? `${envelope.repoUrl}@${envelope.gitRef}` + : envelope.repoUrl; + } + return ""; +} + +function buildFilterEcho(envelope: LeanListFilesEnvelope): string { + const parts: string[] = []; + if (envelope.filter?.pathPrefix) { + parts.push(`path_prefix=${quote(envelope.filter.pathPrefix)}`); + } + if (envelope.filter?.limit !== undefined) { + parts.push(`limit=${envelope.filter.limit}`); + } + return parts.join(" "); +} + +function quote(value: string): string { + return value.includes('"') ? `'${value}'` : `"${value}"`; +} diff --git a/src/shared/read-file-request.test.ts b/src/shared/read-file-request.test.ts index a8828518..ff002d21 100644 --- a/src/shared/read-file-request.test.ts +++ b/src/shared/read-file-request.test.ts @@ -9,7 +9,7 @@ const target: CodeNavigationTarget = { describe("buildReadFileParams — defaults and validation", () => { it("accepts a minimal request and defaults wait to 20000", () => { - const { params, startLineExplicit, endLineExplicit } = buildReadFileParams({ + const { params } = buildReadFileParams({ target, filePath: "src/index.js", }); @@ -17,8 +17,6 @@ describe("buildReadFileParams — defaults and validation", () => { expect(params.startLine).toBeUndefined(); expect(params.endLine).toBeUndefined(); expect(params.waitTimeoutMs).toBe(20000); - expect(startLineExplicit).toBe(false); - expect(endLineExplicit).toBe(false); }); it("trims whitespace around filePath", () => { @@ -36,7 +34,7 @@ describe("buildReadFileParams — defaults and validation", () => { }); it("passes line range through", () => { - const { params, startLineExplicit, endLineExplicit } = buildReadFileParams({ + const { params } = buildReadFileParams({ target, filePath: "src/index.js", startLine: 10, @@ -44,8 +42,6 @@ describe("buildReadFileParams — defaults and validation", () => { }); expect(params.startLine).toBe(10); expect(params.endLine).toBe(40); - expect(startLineExplicit).toBe(true); - expect(endLineExplicit).toBe(true); }); it("accepts open-ended start (end omitted)", () => { diff --git a/src/shared/read-file-request.ts b/src/shared/read-file-request.ts index 792b8333..9af1b52b 100644 --- a/src/shared/read-file-request.ts +++ b/src/shared/read-file-request.ts @@ -24,8 +24,6 @@ export interface ReadFileRequestInput { export interface ReadFileRequestBuildResult { params: ReadFileParams; - startLineExplicit: boolean; - endLineExplicit: boolean; } export function buildReadFileParams( @@ -56,8 +54,6 @@ export function buildReadFileParams( endLine, waitTimeoutMs, }, - startLineExplicit: input.startLine !== undefined, - endLineExplicit: input.endLine !== undefined, }; } diff --git a/src/shared/read-file-response.test.ts b/src/shared/read-file-response.test.ts index c2647698..f90f2765 100644 --- a/src/shared/read-file-response.test.ts +++ b/src/shared/read-file-response.test.ts @@ -101,6 +101,23 @@ describe("buildReadFileSuccessPayload", () => { ); expect(envelope.content).toBe(""); }); + + it("does not auto-populate the hint field — that policy belongs to the MCP handler", () => { + const wideResult: ReadFileResult = { + filePath: "src/big.ts", + language: "typescript", + totalLines: 5000, + startLine: 1, + endLine: 5000, + content: "// big file\n".repeat(5000), + isBinary: false, + }; + const envelope = buildReadFileSuccessPayload(wideResult, { + ...baseOptions, + requestedFilePath: "src/big.ts", + }); + expect(envelope.hint).toBeUndefined(); + }); }); describe("formatReadFileTerminal", () => { diff --git a/src/shared/read-file-response.ts b/src/shared/read-file-response.ts index 1d4248ed..dcaf1234 100644 --- a/src/shared/read-file-response.ts +++ b/src/shared/read-file-response.ts @@ -30,6 +30,14 @@ export interface LeanReadFileEnvelope { content?: string; /** Present and `true` when the file is binary; absent otherwise. */ isBinary?: boolean; + /** + * Optional one-line guidance for the agent. Set by the MCP tool + * handler when it caps the returned span (see + * `src/tools/read-file.ts`). Not auto-populated by this builder — + * the policy of when to nudge an agent belongs to the surface, not + * the response shape. + */ + hint?: string; } export interface BuildReadFilePayloadOptions { @@ -163,6 +171,10 @@ function formatVerboseBody( ); lines.push(`${gutter} ${bodyLines[i]}`); } + if (envelope.hint) { + lines.push(""); + lines.push(dim(envelope.hint, options.useColors)); + } lines.push(""); return lines.join("\n"); } diff --git a/src/shared/unified-search-text.test.ts b/src/shared/unified-search-text.test.ts new file mode 100644 index 00000000..a47fb68e --- /dev/null +++ b/src/shared/unified-search-text.test.ts @@ -0,0 +1,249 @@ +import { describe, expect, it } from "bun:test"; +import type { + UnifiedSearchCompletedPayload, + UnifiedSearchErrorPayload, + UnifiedSearchHitPayload, + UnifiedSearchIncompletePayload, +} from "./unified-search-response.js"; +import { + renderUnifiedSearchError, + renderUnifiedSearchSuccess, +} from "./unified-search-text.js"; + +function codeHit( + overrides: Partial = {}, +): UnifiedSearchHitPayload { + return { + type: "repository_code", + target: "cline/cline@v3.4.2", + title: "applyEdit", + summary: + "Search/replace block parser with fuzzy fallback when exact match fails.", + score: 0.87, + locator: { + registry: "npm", + packageName: "cline", + version: "v3.4.2", + filePath: "src/integrations/diff/strategies/multi-search-replace.ts", + startLine: 142, + endLine: 156, + kind: "function", + }, + ...overrides, + }; +} + +function docsHit(): UnifiedSearchHitPayload { + return { + type: "documentation_page", + target: "aider-AI/aider@v0.55.0", + title: "Edit Formats", + summary: "Compares whole-file, diff-fenced, udiff, and editblock formats.", + score: 0.83, + locator: { + pageId: "aider/edit-formats", + sourceUrl: "https://aider.chat/docs/more/edit-formats.html", + sourceKind: "hosted", + }, + }; +} + +function symbolHit(): UnifiedSearchHitPayload { + return { + type: "repository_symbol", + target: "continuedev/continue@v0.9.42", + title: "diffLines", + summary: "Myers diff core; line-level with O(ND) complexity.", + score: 0.91, + locator: { + filePath: "core/diff/myers.ts", + startLine: 48, + endLine: 112, + qualifiedPath: "core.diff.myers.diffLines", + kind: "function", + category: "callable", + language: "typescript", + }, + }; +} + +function completed( + results: UnifiedSearchHitPayload[], + overrides: Partial = {}, +): UnifiedSearchCompletedPayload { + return { + query: { raw: "diff myers" }, + completed: true, + hasMore: false, + results, + ...overrides, + }; +} + +describe("renderUnifiedSearchSuccess", () => { + it("renders an empty completed envelope with a clear message", () => { + const text = renderUnifiedSearchSuccess(completed([])); + expect(text).toContain("0 hits"); + expect(text).toContain('query="diff myers"'); + expect(text).toContain("No hits."); + }); + + it("renders a single code hit with locator, title, and summary", () => { + const text = renderUnifiedSearchSuccess(completed([codeHit()])); + expect(text).toContain("[1] cline/cline@v3.4.2 code 0.87"); + expect(text).toContain( + " src/integrations/diff/strategies/multi-search-replace.ts:142-156 function", + ); + expect(text).toContain(" applyEdit"); + expect(text).toContain( + " Search/replace block parser with fuzzy fallback when exact match fails.", + ); + }); + + it("uses pageId for documentation hits", () => { + const text = renderUnifiedSearchSuccess(completed([docsHit()])); + expect(text).toContain("[1] aider-AI/aider@v0.55.0 docs 0.83"); + expect(text).toContain(" pageId: aider/edit-formats"); + expect(text).toContain(" Edit Formats"); + }); + + it("renders qualifiedPath alongside file location for symbol hits", () => { + const text = renderUnifiedSearchSuccess(completed([symbolHit()])); + expect(text).toContain("[1] continuedev/continue@v0.9.42 symbol 0.91"); + expect(text).toContain( + " core/diff/myers.ts:48-112 core.diff.myers.diffLines | function", + ); + expect(text).toContain(" diffLines"); + }); + + it("uses ASCII separators throughout (no multi-byte chars)", () => { + const text = renderUnifiedSearchSuccess( + completed([codeHit(), symbolHit()]), + ); + // No common Unicode-Latin1 separator characters in the output. + expect(text).not.toMatch(/[·…—–]/); + }); + + it("emits a truncation hint with offset when hasMore", () => { + const text = renderUnifiedSearchSuccess( + completed([codeHit()], { hasMore: true, nextOffset: 10 }), + ); + expect(text).toContain("More hits available. Pass offset=10"); + }); + + it("falls back to a plain widen hint when nextOffset is missing", () => { + const text = renderUnifiedSearchSuccess( + completed([codeHit()], { hasMore: true }), + ); + expect(text).toContain("More hits available. Pass limit=N to widen."); + }); + + it("renders incomplete payloads with searchRef and progress hint", () => { + const incomplete: UnifiedSearchIncompletePayload = { + query: { raw: "myers" }, + completed: false, + hasMore: false, + results: [codeHit()], + searchRef: "ref_abc-123", + progress: { + status: "INDEXING", + targetsReady: 1, + targetsTotal: 2, + elapsedMs: 8200, + }, + }; + const text = renderUnifiedSearchSuccess(incomplete); + expect(text).toContain("1 partial"); + expect(text).toContain("searchRef=ref_abc-123"); + expect(text).toContain( + "Indexing in progress. Call search_status with searchRef=ref_abc-123", + ); + }); + + it("wraps long summaries at the configured width", () => { + const longSummary = + "This summary is intentionally long enough to force the wrap logic to break it across multiple lines so the renderer's wrap behaviour is verified."; + const text = renderUnifiedSearchSuccess( + completed([codeHit({ summary: longSummary })]), + ); + const summaryLines = text + .split("\n") + .filter( + (line) => line.startsWith(" ") && line.includes("intentionally"), + ); + expect(summaryLines.length).toBeGreaterThanOrEqual(1); + for (const line of text.split("\n")) { + // 4-space indent + content; allow some slack for the wrap target. + expect(line.length).toBeLessThanOrEqual(82); + } + }); + + it("renders source-status notes when the backend reports them", () => { + const text = renderUnifiedSearchSuccess( + completed([codeHit()], { + sourceStatus: [ + { + source: "code", + targetLabel: "npm/cline@v3.4.2", + ignoredFilters: ["fileIntent"], + note: "fileIntent unsupported on code source", + }, + ], + }), + ); + expect(text).toContain("source notes:"); + expect(text).toContain("- code (npm/cline@v3.4.2)"); + expect(text).toContain("ignored=fileIntent"); + }); + + it("separates multiple hits with a blank line", () => { + const text = renderUnifiedSearchSuccess( + completed([codeHit(), docsHit(), symbolHit()]), + ); + const hitHeaders = text.split("\n").filter((line) => /^\[\d\]/.test(line)); + expect(hitHeaders).toHaveLength(3); + expect(text).toContain("[1] cline/cline@v3.4.2"); + expect(text).toContain("[2] aider-AI/aider@v0.55.0"); + expect(text).toContain("[3] continuedev/continue@v0.9.42"); + }); +}); + +describe("renderUnifiedSearchError", () => { + it("renders a basic error", () => { + const error: UnifiedSearchErrorPayload = { + error: "Target is still indexing.", + code: "INDEXING", + retryable: true, + details: { indexingRef: "ref_xyz" }, + }; + const text = renderUnifiedSearchError(error); + expect(text).toContain("search | ERROR | code=INDEXING | retryable"); + expect(text).toContain("Target is still indexing."); + expect(text).toContain("details:"); + expect(text).toContain(" indexingRef: ref_xyz"); + }); + + it("omits retryable marker when not set", () => { + const error: UnifiedSearchErrorPayload = { + error: "Bad request.", + code: "INVALID_ARGUMENT", + }; + const text = renderUnifiedSearchError(error); + expect(text).toBe("search | ERROR | code=INVALID_ARGUMENT\nBad request."); + }); + + it("serialises object detail values via JSON", () => { + const error: UnifiedSearchErrorPayload = { + error: "Indexing.", + code: "INDEXING", + details: { + availableVersions: [ + { version: "4.21.0", ref: "v4.21.0" }, + { version: "4.20.0", ref: "v4.20.0" }, + ], + }, + }; + const text = renderUnifiedSearchError(error); + expect(text).toContain('"version":"4.21.0"'); + }); +}); diff --git a/src/shared/unified-search-text.ts b/src/shared/unified-search-text.ts new file mode 100644 index 00000000..1618b56e --- /dev/null +++ b/src/shared/unified-search-text.ts @@ -0,0 +1,258 @@ +/** + * Line-oriented text renderer for unified `search` MCP responses. + * + * Designed for agent context efficiency: roughly 3-5 lines per hit + * and no JSON scaffolding. This is the tool's default response + * format — programmatic / parity callers opt into the structured + * JSON envelope by passing `format: "json"`. + * + * ASCII-only output — separators tokenize cleanly across BPE + * variants, and there are no Unicode characters that require + * client-side escaping. + * + * Format is a public contract — locked with snapshot-style tests in + * `unified-search-text.test.ts`. Update the spec in + * `docs/implementation/tools.md` when changing the format. + */ + +import type { + UnifiedSearchCompletedPayload, + UnifiedSearchErrorPayload, + UnifiedSearchHitPayload, + UnifiedSearchIncompletePayload, +} from "./unified-search-response.js"; + +const SUMMARY_WRAP_WIDTH = 76; +const SEP = " | "; + +type SearchSuccessPayload = + | UnifiedSearchCompletedPayload + | UnifiedSearchIncompletePayload; + +/** Render a successful unified-search payload as line-oriented text. */ +export function renderUnifiedSearchSuccess( + payload: SearchSuccessPayload, +): string { + const lines: string[] = []; + lines.push(buildHeader(payload)); + lines.push(""); + + if (payload.results.length === 0) { + lines.push(payload.completed ? "No hits." : "No hits yet - indexing."); + } else { + payload.results.forEach((hit, idx) => { + if (idx > 0) lines.push(""); + appendHit(lines, idx + 1, hit); + }); + } + + const trailer = buildTrailer(payload); + if (trailer.length > 0) { + lines.push(""); + for (const line of trailer) lines.push(line); + } + + return lines.join("\n"); +} + +/** Render an error envelope as compact text. */ +export function renderUnifiedSearchError( + payload: UnifiedSearchErrorPayload, +): string { + const lines: string[] = []; + const header = `search${SEP}ERROR${SEP}code=${payload.code}${ + payload.retryable ? `${SEP}retryable` : "" + }`; + lines.push(header); + lines.push(payload.error); + + if (payload.details && Object.keys(payload.details).length > 0) { + lines.push(""); + lines.push("details:"); + for (const [key, value] of Object.entries(payload.details)) { + lines.push(` ${key}: ${formatDetailValue(value)}`); + } + } + return lines.join("\n"); +} + +function buildHeader(payload: SearchSuccessPayload): string { + const count = payload.results.length; + const status = payload.completed + ? `${count} hit${count === 1 ? "" : "s"}` + : `${count} partial`; + const parts = [`search${SEP}${status}`]; + parts.push(`query=${quote(payload.query.raw)}`); + if (!payload.completed) { + parts.push(`searchRef=${payload.searchRef}`); + } + return parts.join(SEP); +} + +function appendHit( + lines: string[], + index: number, + hit: UnifiedSearchHitPayload, +): void { + const headerParts: string[] = [hit.target, shortType(hit.type)]; + if (typeof hit.score === "number") { + headerParts.push(formatScore(hit.score)); + } + lines.push(`[${index}] ${headerParts.join(" ")}`); + + const locator = buildLocatorLine(hit); + if (locator) lines.push(` ${locator}`); + + // Title is suppressed when it's literally the locator we just + // printed; the response builder already drops `qualifiedPath` when + // it equals `title`, so we don't double-check that here. + if (hit.title && hit.title !== hit.locator.filePath) { + lines.push(` ${hit.title}`); + } + + if (hit.summary) { + for (const wrapped of wrapText(hit.summary, SUMMARY_WRAP_WIDTH)) { + lines.push(` ${wrapped}`); + } + } +} + +/** + * Compact, agent-friendly type label. + * + * Backend types are uppercase enum-style; the JSON envelope already + * lowercases them. Text mode further compacts to a single token so a + * reader can scan the third column quickly. + */ +function shortType(type: string): string { + switch (type) { + case "repository_code": + return "code"; + case "repository_symbol": + return "symbol"; + case "documentation_page": + return "docs"; + case "repository_doc": + return "repo-docs"; + default: + return type; + } +} + +function buildLocatorLine(hit: UnifiedSearchHitPayload): string { + const loc = hit.locator; + if (loc.filePath) { + let line = `${loc.filePath}${formatLineRange(loc.startLine, loc.endLine)}`; + const tail: string[] = []; + if (loc.qualifiedPath) tail.push(loc.qualifiedPath); + if (loc.kind) tail.push(loc.kind); + if (tail.length > 0) line += ` ${tail.join(SEP)}`; + return line; + } + if (loc.pageId) return `pageId: ${loc.pageId}`; + if (loc.sourceUrl) return loc.sourceUrl; + return ""; +} + +function formatLineRange(start?: number, end?: number): string { + if (typeof start !== "number") return ""; + if (typeof end !== "number" || end === start) return `:${start}`; + return `:${start}-${end}`; +} + +function formatScore(score: number): string { + return score.toFixed(2); +} + +function buildTrailer(payload: SearchSuccessPayload): string[] { + const lines: string[] = []; + + if (payload.hasMore) { + const nextOffsetHint = + typeof payload.nextOffset === "number" + ? ` Pass offset=${payload.nextOffset} for the next page or limit=N to widen.` + : " Pass limit=N to widen."; + lines.push(`More hits available.${nextOffsetHint}`); + } + + if (!payload.completed && payload.searchRef) { + lines.push( + `Indexing in progress. Call search_status with searchRef=${payload.searchRef} to follow up.`, + ); + } + + if (payload.sourceStatus && payload.sourceStatus.length > 0) { + lines.push("source notes:"); + for (const entry of payload.sourceStatus) { + lines.push(` - ${formatSourceStatus(entry)}`); + } + } + + return lines; +} + +function formatSourceStatus(entry: { + source: string; + targetLabel: string; + indexingStatus?: string; + codeIndexState?: string; + ignoredFilters?: string[]; + incompatibleFilters?: string[]; + ignoredQueryFeatures?: string[]; + incompatibleQueryFeatures?: string[]; + note?: string; +}): string { + const parts: string[] = [`${entry.source} (${entry.targetLabel})`]; + if (entry.indexingStatus) parts.push(`indexing=${entry.indexingStatus}`); + if (entry.codeIndexState) parts.push(`codeIndex=${entry.codeIndexState}`); + if (entry.ignoredFilters?.length) { + parts.push(`ignored=${entry.ignoredFilters.join(",")}`); + } + if (entry.incompatibleFilters?.length) { + parts.push(`incompatible=${entry.incompatibleFilters.join(",")}`); + } + if (entry.ignoredQueryFeatures?.length) { + parts.push(`ignoredQuery=${entry.ignoredQueryFeatures.join(",")}`); + } + if (entry.incompatibleQueryFeatures?.length) { + parts.push( + `incompatibleQuery=${entry.incompatibleQueryFeatures.join(",")}`, + ); + } + if (entry.note) parts.push(entry.note); + return parts.join(SEP); +} + +function quote(value: string): string { + // Use single quotes when the value already contains a double quote; + // agents read either form. JSON-escape would be over-engineering for + // a header. + return value.includes('"') ? `'${value}'` : `"${value}"`; +} + +function formatDetailValue(value: unknown): string { + if (value === null || value === undefined) return ""; + if (typeof value === "string") return value; + if (typeof value === "number" || typeof value === "boolean") + return String(value); + return JSON.stringify(value); +} + +function wrapText(text: string, width: number): string[] { + const lines: string[] = []; + for (const paragraph of text.split(/\n/)) { + if (paragraph.length === 0) { + lines.push(""); + continue; + } + let remaining = paragraph.trim(); + while (remaining.length > width) { + let breakAt = remaining.lastIndexOf(" ", width); + if (breakAt <= 0) breakAt = width; + lines.push(remaining.slice(0, breakAt).trimEnd()); + remaining = remaining.slice(breakAt).trimStart(); + } + if (remaining.length > 0) lines.push(remaining); + } + return lines; +} diff --git a/src/tools/grep-repo-parity.test.ts b/src/tools/grep-repo-parity.test.ts index 58cc1d9b..46d5a9b6 100644 --- a/src/tools/grep-repo-parity.test.ts +++ b/src/tools/grep-repo-parity.test.ts @@ -81,6 +81,7 @@ interface McpArgs { pattern: string; path_prefix?: string; wait_timeout_ms?: number; + format?: "json" | "text" | "text-v1"; } async function mcpJson( @@ -91,7 +92,10 @@ async function mcpJson( grepRepoMock ? { grepRepo: grepRepoMock as never } : {}, ); const tool = createGrepRepoTool(service); - const result = await tool.handler(args, {}); + // Parity is asserted against the JSON envelope. The MCP default is + // text-v1, so this helper opts into JSON to match the CLI `--json` + // payload shape. + const result = await tool.handler({ ...args, format: "json" }, {}); return JSON.parse(result.content[0]?.text ?? ""); } diff --git a/src/tools/grep-repo.test.ts b/src/tools/grep-repo.test.ts index cbde27bf..057fb90d 100644 --- a/src/tools/grep-repo.test.ts +++ b/src/tools/grep-repo.test.ts @@ -27,6 +27,7 @@ describe("createGrepRepoTool — metadata", () => { "exclude_doc_files", "exclude_test_files", "extensions", + "format", "globs", "max_matches", "max_matches_per_file", @@ -112,12 +113,13 @@ describe("createGrepRepoTool — happy path", () => { ); }); - it("emits the new envelope shape", async () => { + it("emits the JSON envelope shape when format=json", async () => { const tool = createGrepRepoTool(createMockCodeNavigationService()); const result = await tool.handler( { target: { registry: "npm", package_name: "express" }, pattern: "middleware", + format: "json", }, {}, ); @@ -140,6 +142,53 @@ describe("createGrepRepoTool — happy path", () => { }); }); +describe("createGrepRepoTool — text format", () => { + it("defaults to text output when format is omitted", async () => { + const tool = createGrepRepoTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + pattern: "middleware", + }, + {}, + ); + expect(result.isError).toBeUndefined(); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_grep | 1 match in 1 file"); + expect(text).toContain('pattern="middleware"'); + expect(text).toContain("src/index.js (1)"); + expect(() => JSON.parse(text)).toThrow(); + }); + + it("renders text output when format=text-v1", async () => { + const tool = createGrepRepoTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + pattern: "middleware", + format: "text-v1", + }, + {}, + ); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_grep | "); + }); + + it("accepts format=text as an alias for text-v1", async () => { + const tool = createGrepRepoTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + pattern: "middleware", + format: "text", + }, + {}, + ); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_grep | "); + }); +}); + describe("createGrepRepoTool — validation errors", () => { it("returns INVALID_ARGUMENT for empty pattern", async () => { const tool = createGrepRepoTool(createMockCodeNavigationService()); diff --git a/src/tools/grep-repo.ts b/src/tools/grep-repo.ts index 950f3b0c..70388b51 100644 --- a/src/tools/grep-repo.ts +++ b/src/tools/grep-repo.ts @@ -8,6 +8,7 @@ import { GREP_REPO_SYMBOL_FIELDS, GREP_REPO_SYMBOL_FIELDS_NOTE, type GrepRepoSymbolField, + renderGrepRepoText, } from "../shared/index.js"; import { toPkgseerRegistryLowercase } from "../shared/pkgseer-registry.js"; import { @@ -36,6 +37,7 @@ export interface GrepRepoArgs { cursor?: string; symbol_fields?: GrepRepoSymbolField[]; wait_timeout_ms?: number; + format?: "json" | "text" | "text-v1"; } const schema = { @@ -78,13 +80,20 @@ const schema = { .optional() .describe(GREP_REPO_SYMBOL_FIELDS_NOTE), wait_timeout_ms: z.number().optional(), + format: z + .enum(["json", "text", "text-v1"]) + .optional() + .describe( + 'Response format. Default `text-v1` — compact line-oriented output (matches grouped by file with grep -A/-B notation for context). Pass `format: "json"` for the structured envelope. `text` is an alias for `text-v1`. Errors stay JSON-formatted in either mode for now.', + ), }; const DESCRIPTION = "Deterministic text grep over indexed dependency and repository source files. " + "Use this when you know the text pattern you want; use `search` for discovery. " + - "Whole-target grep is the default. Narrow with `path`, `path_prefix`, `globs`, or `extensions`. " + - "Matches chain directly into `code_read` via `matches[].filePath`."; + "Whole-target grep is the default — narrow with `path`, `path_prefix`, `globs`, or `extensions` to keep responses small. " + + 'Default response is a compact line-oriented listing (`format: "text-v1"`); pass `format: "json"` for the structured envelope. ' + + "Matches chain directly into `code_read` (the `path` and `line` from each match feed straight into `start_line` / `end_line`)."; export function createGrepRepoTool( service: CodeNavigationService, @@ -146,6 +155,9 @@ export function createGrepRepoTool( excludeTestFiles: build.params.excludeTestFiles, explicit: build.explicit, }); + if (isTextFormat(args.format)) { + return textResult(renderGrepRepoText(payload)); + } return textResult(JSON.stringify(payload)); } catch (error) { const mapped = mapCodeNavigationError(error); @@ -161,3 +173,11 @@ export function createGrepRepoTool( }, }; } + +/** + * Default response format is text-v1; programmatic callers opt into + * JSON explicitly via `format: "json"`. + */ +function isTextFormat(format: GrepRepoArgs["format"]): boolean { + return format === undefined || format === "text" || format === "text-v1"; +} diff --git a/src/tools/list-files-parity.test.ts b/src/tools/list-files-parity.test.ts index 3eb4d944..21d07896 100644 --- a/src/tools/list-files-parity.test.ts +++ b/src/tools/list-files-parity.test.ts @@ -1,6 +1,8 @@ // PARITY TEST — enforces: -// PARITY-JSON-KEYS CLI --json output and MCP text payload parse to -// deepEqual JSON objects for equivalent inputs. +// PARITY-JSON-KEYS CLI --json output and MCP `format: "json"` payload +// parse to deepEqual JSON objects for equivalent +// inputs. The MCP default is `text-v1`; this helper +// opts into JSON to compare like-for-like. // PARITY-ERROR-ENVELOPE Both surfaces emit { error, code, retryable, details? }. import { describe, expect, it, mock, spyOn } from "bun:test"; @@ -84,6 +86,7 @@ interface McpArgs { path_prefix?: string; limit?: number; wait_timeout_ms?: number; + format?: "json" | "text" | "text-v1"; } async function mcpJson( @@ -94,7 +97,10 @@ async function mcpJson( listFilesMock ? { listFiles: listFilesMock as never } : {}, ); const tool = createListFilesTool(service); - const result = await tool.handler(args, {}); + // Parity is asserted against the JSON envelope. The MCP default is + // text-v1, so this helper opts into JSON to match the CLI `--json` + // payload shape. + const result = await tool.handler({ ...args, format: "json" }, {}); return JSON.parse(result.content[0]?.text ?? ""); } diff --git a/src/tools/list-files.test.ts b/src/tools/list-files.test.ts index 4f394d5a..f5db2ab4 100644 --- a/src/tools/list-files.test.ts +++ b/src/tools/list-files.test.ts @@ -19,6 +19,7 @@ describe("createListFilesTool — metadata", () => { expect(tool.name).toBe("code_files"); expect(tool.description).toContain("List files in an indexed dependency"); expect(Object.keys(tool.schema).sort()).toEqual([ + "format", "limit", "path_prefix", "target", @@ -51,7 +52,7 @@ describe("createListFilesTool — happy path", () => { it("emits the envelope with files, total, hasMore, resolution, indexedVersion", async () => { const tool = createListFilesTool(createMockCodeNavigationService()); const result = await tool.handler( - { target: { registry: "npm", package_name: "express" } }, + { target: { registry: "npm", package_name: "express" }, format: "json" }, {}, ); expect(result.isError).toBeUndefined(); @@ -81,6 +82,7 @@ describe("createListFilesTool — happy path", () => { repo_url: "https://github.com/expressjs/express", git_ref: "main", }, + format: "json", }, {}, ); @@ -102,6 +104,7 @@ describe("createListFilesTool — happy path", () => { { target: { registry: "npm", package_name: "express" }, path_prefix: "src/", + format: "json", }, {}, ); @@ -114,7 +117,7 @@ describe("createListFilesTool — happy path", () => { it("omits filter when caller only used defaults", async () => { const tool = createListFilesTool(createMockCodeNavigationService()); const result = await tool.handler( - { target: { registry: "npm", package_name: "express" } }, + { target: { registry: "npm", package_name: "express" }, format: "json" }, {}, ); const payload = parseText(result) as { filter?: unknown }; @@ -210,3 +213,62 @@ describe("createListFilesTool — service errors", () => { expect(payload.code).toBe("NOT_FOUND"); }); }); + +describe("createListFilesTool — text format", () => { + it("defaults to text output when format is omitted", async () => { + const tool = createListFilesTool(createMockCodeNavigationService()); + const result = await tool.handler( + { target: { registry: "npm", package_name: "express" } }, + {}, + ); + expect(result.isError).toBeUndefined(); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_files | 2 paths"); + // Confirm text payload is not valid JSON (proves text default). + expect(() => JSON.parse(text)).toThrow(); + }); + + it("returns line-oriented text when format=text-v1", async () => { + const tool = createListFilesTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + format: "text-v1", + }, + {}, + ); + expect(result.isError).toBeUndefined(); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_files | 2 paths | npm:express@v5.2.1"); + expect(text).toContain("src/index.js"); + // Confirm the text payload is not valid JSON. + expect(() => JSON.parse(text)).toThrow(); + }); + + it("accepts format=text as an alias for text-v1", async () => { + const tool = createListFilesTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + format: "text", + }, + {}, + ); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("code_files | 2 paths"); + }); + + it("keeps JSON envelope when format=json (explicit)", async () => { + const tool = createListFilesTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + format: "json", + }, + {}, + ); + const payload = parseText(result) as { registry: string; total: number }; + expect(payload.registry).toBe("npm"); + expect(payload.total).toBe(2); + }); +}); diff --git a/src/tools/list-files.ts b/src/tools/list-files.ts index 5f11f959..722bed71 100644 --- a/src/tools/list-files.ts +++ b/src/tools/list-files.ts @@ -3,6 +3,7 @@ import type { CodeNavigationService } from "../services/index.js"; import { mapCodeNavigationError } from "../shared/code-navigation-error-map.js"; import { buildListFilesParams } from "../shared/list-files-request.js"; import { buildListFilesSuccessPayload } from "../shared/list-files-response.js"; +import { renderListFilesText } from "../shared/list-files-text.js"; import { toPkgseerRegistryLowercase } from "../shared/pkgseer-registry.js"; import { type CodeTargetArg, @@ -16,6 +17,7 @@ export interface ListFilesArgs { path_prefix?: string; limit?: number; wait_timeout_ms?: number; + format?: "json" | "text" | "text-v1"; } const schema = { @@ -38,20 +40,27 @@ const schema = { .describe( "Max milliseconds to wait for indexing (0–60000, default 20000). On an `INDEXING` error envelope, retry with a longer timeout or pass a version from `details.availableVersions`.", ), + format: z + .enum(["json", "text", "text-v1"]) + .optional() + .describe( + 'Response format. Default `text-v1` — compact paths-only listing tuned for agent context efficiency. Pass `format: "json"` for the structured envelope. `text` is an alias for `text-v1`. Errors stay JSON-formatted in either mode for now.', + ), }; const DESCRIPTION = - "List files in an indexed dependency. Response: " + - "`{total, hasMore, files: [{path, name, language, fileType, byteSize}], " + - "resolution, indexedVersion}`. Address via `target.registry` + " + - "`target.package_name` (package scope) or `target.repo_url` + " + - "`target.git_ref` (repo scope), mutually exclusive. `path_prefix` " + - "is a literal directory prefix — it does NOT accept globs " + - "(`*.ts`) or extension filters. The returned `path` values feed " + - "directly into `code_read` and help scope `code_grep`. Returns an `INDEXING` " + - "error envelope when the dependency is being indexed on-demand — " + - "retry with a longer `wait_timeout_ms` or use a version from " + - "`details.availableVersions`."; + "List files in an indexed dependency. Default response is a compact " + + 'paths-only listing (`format: "text-v1"`); pass `format: "json"` ' + + "for the structured envelope `{total, hasMore, files: [{path, name, " + + "language, fileType, byteSize}], resolution, indexedVersion}`. " + + "Address via `target.registry` + `target.package_name` (package " + + "scope) or `target.repo_url` + `target.git_ref` (repo scope), " + + "mutually exclusive. `path_prefix` is a literal directory prefix — " + + "it does NOT accept globs (`*.ts`) or extension filters. The " + + "returned paths feed directly into `code_read` and help scope " + + "`code_grep`. Returns an `INDEXING` error envelope when the " + + "dependency is being indexed on-demand — retry with a longer " + + "`wait_timeout_ms` or use a version from `details.availableVersions`."; export function createListFilesTool( service: CodeNavigationService, @@ -85,6 +94,9 @@ export function createListFilesTool( pathPrefix: build.params.pathPrefix, limit: build.params.limit, }); + if (isTextFormat(args.format)) { + return textResult(renderListFilesText(payload)); + } return textResult(JSON.stringify(payload)); } catch (error) { const mapped = mapCodeNavigationError(error); @@ -100,3 +112,11 @@ export function createListFilesTool( }, }; } + +/** + * Default response format is text-v1; programmatic callers opt into + * JSON explicitly via `format: "json"`. + */ +function isTextFormat(format: ListFilesArgs["format"]): boolean { + return format === undefined || format === "text" || format === "text-v1"; +} diff --git a/src/tools/read-file-parity.test.ts b/src/tools/read-file-parity.test.ts index ba1faaff..0f576d55 100644 --- a/src/tools/read-file-parity.test.ts +++ b/src/tools/read-file-parity.test.ts @@ -91,7 +91,18 @@ async function mcpJson( ); const tool = createReadFileTool(service); const result = await tool.handler(args, {}); - return JSON.parse(result.content[0]?.text ?? ""); + const parsed = JSON.parse(result.content[0]?.text ?? "") as Record< + string, + unknown + >; + // The MCP surface adds a `hint` field when its per-call span cap + // truncates the request. The cap is intentionally MCP-only — the + // CLI command path honors arbitrary ranges so humans can pipe + // whole files. Strip the field here so the envelope shapes match + // for parity comparison; cap behavior is covered by + // `read-file.test.ts`. + delete parsed.hint; + return parsed; } describe("read_file parity", () => { diff --git a/src/tools/read-file.test.ts b/src/tools/read-file.test.ts index 4ec866cb..e63eef4b 100644 --- a/src/tools/read-file.test.ts +++ b/src/tools/read-file.test.ts @@ -227,3 +227,241 @@ describe("createReadFileTool — service errors", () => { expect((parseText(result) as { code: string }).code).toBe("INDEXING"); }); }); + +describe("createReadFileTool — span cap", () => { + it("clamps no-range request to 1..MCP_READ_MAX_SPAN before calling backend", async () => { + const readFile = mock(() => Promise.resolve(defaultReadFileResult)); + const tool = createReadFileTool( + createMockCodeNavigationService({ readFile }), + ); + await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + }, + {}, + ); + const calls = readFile.mock.calls as unknown as Array< + [{ startLine?: number; endLine?: number }] + >; + expect(calls[0]?.[0]?.startLine).toBe(1); + expect(calls[0]?.[0]?.endLine).toBe(150); + }); + + it("clamps a wide explicit range to start..start+149", async () => { + const readFile = mock(() => Promise.resolve(defaultReadFileResult)); + const tool = createReadFileTool( + createMockCodeNavigationService({ readFile }), + ); + await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 200, + end_line: 600, + }, + {}, + ); + const calls = readFile.mock.calls as unknown as Array< + [{ startLine?: number; endLine?: number }] + >; + expect(calls[0]?.[0]?.startLine).toBe(200); + expect(calls[0]?.[0]?.endLine).toBe(349); + }); + + it("clamps start-only request to start..start+149", async () => { + const readFile = mock(() => Promise.resolve(defaultReadFileResult)); + const tool = createReadFileTool( + createMockCodeNavigationService({ readFile }), + ); + await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 100, + }, + {}, + ); + const calls = readFile.mock.calls as unknown as Array< + [{ startLine?: number; endLine?: number }] + >; + expect(calls[0]?.[0]?.startLine).toBe(100); + expect(calls[0]?.[0]?.endLine).toBe(249); + }); + + it("does not clamp ranges within the cap", async () => { + const readFile = mock(() => Promise.resolve(defaultReadFileResult)); + const tool = createReadFileTool( + createMockCodeNavigationService({ readFile }), + ); + await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 10, + end_line: 80, + }, + {}, + ); + const calls = readFile.mock.calls as unknown as Array< + [{ startLine?: number; endLine?: number }] + >; + expect(calls[0]?.[0]?.startLine).toBe(10); + expect(calls[0]?.[0]?.endLine).toBe(80); + }); + + // Hint tests need a backend that actually returns a >150-line span; + // the default 5-line mock would make the cap a no-op and the hint + // would (correctly) be suppressed. + function wideFileMock( + overrides: { + startLine?: number; + endLine?: number; + totalLines?: number; + } = {}, + ) { + const start = overrides.startLine ?? 1; + const end = overrides.endLine ?? 150; + const total = overrides.totalLines ?? 5000; + return mock(() => + Promise.resolve({ + filePath: "src/index.js", + language: "javascript", + totalLines: total, + startLine: start, + endLine: end, + content: "// big file\n".repeat(end - start + 1), + isBinary: false, + }), + ); + } + + it("emits hint with actual returned range and original request when capping", async () => { + const tool = createReadFileTool( + createMockCodeNavigationService({ + readFile: wideFileMock({ + startLine: 1, + endLine: 150, + totalLines: 5000, + }), + }), + ); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 1, + end_line: 600, + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toBeDefined(); + // Returned range comes from the payload (1-150/5000), not the + // pre-clamp request — pre-Codex-fix this rendered the impossible + // "1-150/5" against the 5-line mock. + expect(payload.hint).toContain("Returned lines 1-150/5000"); + expect(payload.hint).toContain("MCP cap: 150 lines"); + expect(payload.hint).toContain("you requested lines 1-600"); + expect(payload.hint).toContain("80-150 lines"); + expect(payload.hint).toContain("retry also costs context"); + }); + + it("emits hint with 'no range' wording when caller passed nothing", async () => { + const tool = createReadFileTool( + createMockCodeNavigationService({ + readFile: wideFileMock(), + }), + ); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toContain("you requested no range"); + }); + + it("does not emit hint when range is within the cap", async () => { + const tool = createReadFileTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 10, + end_line: 80, + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toBeUndefined(); + }); + + it("does not emit hint when the cap was a no-op (file fits within cap)", async () => { + // Default 5-line mock: caller passed no range, cap clamped the + // request to 1..150 but the backend returned the whole 5-line + // file. No actual truncation happened — hint would point at + // nonexistent lines (Codex review P2). + const tool = createReadFileTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toBeUndefined(); + }); + + it("does not emit hint when the returned range reaches end of file", async () => { + // Caller asked 100-600 against a 200-line file. Cap clamped the + // request to 100..249, backend returned 100..200 (EOF). The + // agent has all the available content already; hint would just + // suggest narrower windows that wouldn't help. + const tool = createReadFileTool( + createMockCodeNavigationService({ + readFile: wideFileMock({ + startLine: 100, + endLine: 200, + totalLines: 200, + }), + }), + ); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "src/index.js", + start_line: 100, + end_line: 600, + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toBeUndefined(); + }); + + it("does not emit hint for binary files even when no range was supplied", async () => { + const tool = createReadFileTool( + createMockCodeNavigationService({ + readFile: mock(() => + Promise.resolve({ + filePath: "assets/logo.png", + isBinary: true, + }), + ), + }), + ); + const result = await tool.handler( + { + target: { registry: "npm", package_name: "express" }, + path: "assets/logo.png", + }, + {}, + ); + const payload = parseText(result) as { hint?: string }; + expect(payload.hint).toBeUndefined(); + }); +}); diff --git a/src/tools/read-file.ts b/src/tools/read-file.ts index 3fe1cd11..969913c8 100644 --- a/src/tools/read-file.ts +++ b/src/tools/read-file.ts @@ -3,7 +3,10 @@ import type { CodeNavigationService } from "../services/index.js"; import { mapCodeNavigationError } from "../shared/code-navigation-error-map.js"; import { toPkgseerRegistryLowercase } from "../shared/pkgseer-registry.js"; import { buildReadFileParams } from "../shared/read-file-request.js"; -import { buildReadFileSuccessPayload } from "../shared/read-file-response.js"; +import { + buildReadFileSuccessPayload, + type LeanReadFileEnvelope, +} from "../shared/read-file-response.js"; import { type CodeTargetArg, codeTargetSchema, @@ -11,6 +14,18 @@ import { } from "./code-navigation-shared.js"; import { errorResult, type ToolDefinition, textResult } from "./types.js"; +/** + * Maximum line span the MCP `code_read` tool will return in one call. + * + * Real session traces showed agents requesting 300-600 line windows + * (and occasionally unbounded full-file reads) which dominated + * context cost. The cap forces agents to pick a focused window — + * typical 80-150 lines around a known symbol from `search` or + * `code_grep` results. CLI command `githits code read` bypasses this + * cap so humans piping a whole file to disk still work. + */ +export const MCP_READ_MAX_SPAN = 150; + export interface ReadFileArgs { target: CodeTargetArg; path: string; @@ -29,12 +44,14 @@ const schema = { start_line: z .number() .optional() - .describe("Starting line (1-indexed). Omit for the full file from line 1."), + .describe( + `Starting line (1-indexed). Omit to start at line 1. The MCP surface caps any single read at ${MCP_READ_MAX_SPAN} lines — pick a focused window from your prior \`search\` / \`code_grep\` hit.`, + ), end_line: z .number() .optional() .describe( - "Ending line (inclusive). Omit for end of file. Must be ≥ `start_line` when both are set.", + `Ending line (inclusive). Must be ≥ \`start_line\` when both are set. Omitting it implies \`start_line + ${MCP_READ_MAX_SPAN - 1}\` because the MCP surface caps each read at ${MCP_READ_MAX_SPAN} lines.`, ), wait_timeout_ms: z .number() @@ -45,18 +62,64 @@ const schema = { }; const DESCRIPTION = - "Read a file from an indexed dependency. Default returns the full " + - "file; use `start_line` / `end_line` for a bounded range. Response: " + - "`{path, language, totalLines, startLine, endLine, content, " + - "isBinary}`. Binary files set `isBinary: true` and omit `content` — " + - "agents branch on the flag rather than checking null. Pass the same " + - "`path` emitted by `code_files`. Address via " + - "`target.registry` + `target.package_name` (package scope) or " + - "`target.repo_url` + `target.git_ref` (repo scope), mutually " + - "exclusive. On `INDEXING` retry with a longer `wait_timeout_ms`. " + - "When the path doesn't resolve the response is a `NOT_FOUND` (or " + - "`FILE_NOT_FOUND`) error — call `code_files` to discover the " + - "actual paths."; + "Read a file from an indexed dependency. **MCP cap: " + + `${MCP_READ_MAX_SPAN} lines per call** — broader requests (or no ` + + `range) silently truncate to the first ${MCP_READ_MAX_SPAN} lines ` + + "from your start, with a `hint` describing what was returned vs. " + + "requested. Pick a focused window from a `search` / `code_grep` " + + "match. Response: `{path, language, totalLines, startLine, endLine, " + + "content, isBinary, hint?}`. Binary files set `isBinary: true` and " + + "omit `content`. Pass the same `path` emitted by `code_files`. " + + "Address via `target.registry` + `target.package_name` (package " + + "scope) or `target.repo_url` + `target.git_ref` (repo scope), " + + "mutually exclusive. On `INDEXING` retry with a longer " + + "`wait_timeout_ms`. On `NOT_FOUND` / `FILE_NOT_FOUND` call " + + "`code_files` to discover the actual path."; + +interface BoundedRange { + startLine: number; + endLine: number; + capped: boolean; +} + +/** + * Compute the effective `(startLine, endLine)` for the backend call, + * enforcing the MCP per-call span cap. + * + * - No range supplied: `1..MCP_READ_MAX_SPAN`. + * - `start_line` only: `start..start + MCP_READ_MAX_SPAN - 1`. + * - `end_line` only: treat start as 1; if span > cap, clamp. + * - Both supplied with span ≤ cap: untouched. + * - Both supplied with span > cap: clamp to `start..start + cap - 1`. + * + * The cap is enforced before the backend call so the service does not + * have to transfer bytes that will be discarded. + */ +export function deriveBoundedRange( + startLine: number | undefined, + endLine: number | undefined, +): BoundedRange { + const start = startLine ?? 1; + + if (endLine === undefined) { + return { + startLine: start, + endLine: start + MCP_READ_MAX_SPAN - 1, + capped: true, + }; + } + + const span = endLine - start + 1; + if (span > MCP_READ_MAX_SPAN) { + return { + startLine: start, + endLine: start + MCP_READ_MAX_SPAN - 1, + capped: true, + }; + } + + return { startLine: start, endLine, capped: false }; +} export function createReadFileTool( service: CodeNavigationService, @@ -71,11 +134,15 @@ export function createReadFileTool( if ("content" in target) return target; try { + // Cap before the backend call so we don't transfer bytes only + // to throw them away. CLI surface bypasses this — see the + // MCP_READ_MAX_SPAN doc-comment for rationale. + const bounded = deriveBoundedRange(args.start_line, args.end_line); const build = buildReadFileParams({ target, filePath: args.path, - startLine: args.start_line, - endLine: args.end_line, + startLine: bounded.startLine, + endLine: bounded.endLine, waitTimeoutMs: args.wait_timeout_ms, }); const result = await service.readFile(build.params); @@ -88,6 +155,15 @@ export function createReadFileTool( gitRef: target.gitRef, requestedFilePath: build.params.filePath, }); + + if (shouldEmitCappedHint(bounded, payload)) { + payload.hint = buildCappedHint( + payload, + args.start_line, + args.end_line, + ); + } + return textResult(JSON.stringify(payload)); } catch (error) { const mapped = mapCodeNavigationError(error); @@ -103,3 +179,59 @@ export function createReadFileTool( }, }; } + +/** + * Whether to emit the cap hint. + * + * The hint is only useful when the response was actually truncated — + * i.e., the caller's intent ran past the end of what came back. If + * the caller asked for the whole file but the file fits within the + * cap, the response is the whole file and the hint would point at + * lines that don't exist (real bug found by Codex review). + * + * Suppression cases: + * - Cap clamp logic didn't fire at all. + * - Binary file (hint is irrelevant). + * - Backend didn't echo `endLine` / `totalLines` (we can't tell). + * - The returned end IS the end of the file. + */ +function shouldEmitCappedHint( + bounded: BoundedRange, + payload: LeanReadFileEnvelope, +): boolean { + if (!bounded.capped) return false; + if (payload.isBinary) return false; + if (payload.endLine === undefined) return false; + if (payload.totalLines === undefined) return false; + return payload.endLine < payload.totalLines; +} + +function buildCappedHint( + payload: LeanReadFileEnvelope, + originalStart: number | undefined, + originalEnd: number | undefined, +): string { + const requested = describeRequest(originalStart, originalEnd); + return ( + `Returned lines ${payload.startLine}-${payload.endLine}/${payload.totalLines} ` + + `(MCP cap: ${MCP_READ_MAX_SPAN} lines per call; you requested ${requested}). ` + + `Pick a focused start_line/end_line window — typical 80-150 lines around a search/code_grep match. ` + + `Each retry also costs context, so aim for one well-sized read.` + ); +} + +function describeRequest( + originalStart: number | undefined, + originalEnd: number | undefined, +): string { + if (originalStart === undefined && originalEnd === undefined) { + return "no range"; + } + if (originalEnd === undefined) { + return `start_line=${originalStart}, no end_line`; + } + if (originalStart === undefined) { + return `end_line=${originalEnd}, no start_line`; + } + return `lines ${originalStart}-${originalEnd}`; +} diff --git a/src/tools/search.test.ts b/src/tools/search.test.ts index 337e3e3a..39d45d2b 100644 --- a/src/tools/search.test.ts +++ b/src/tools/search.test.ts @@ -14,6 +14,7 @@ describe("searchTool", () => { { query: "router middleware", target: { registry: "npm", package_name: "express" }, + format: "json", }, {}, ); @@ -123,6 +124,7 @@ describe("searchTool", () => { { query: "middleware", target: { registry: "npm", package_name: "express" }, + format: "json", }, {}, ); @@ -138,4 +140,65 @@ describe("searchTool", () => { expect(payload.results[0]).not.toHaveProperty("followUp"); expect(payload.results[0]).not.toHaveProperty("alternateFollowUps"); }); + + it("defaults to text output when format is omitted", async () => { + const tool = createSearchTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + query: "router middleware", + target: { registry: "npm", package_name: "express" }, + }, + {}, + ); + expect(result.isError).toBeUndefined(); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("search | "); + expect(() => JSON.parse(text)).toThrow(); + }); + + it("renders text output when format=text-v1", async () => { + const tool = createSearchTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + query: "router middleware", + target: { registry: "npm", package_name: "express" }, + format: "text-v1", + }, + {}, + ); + expect(result.isError).toBeUndefined(); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("search | "); + expect(text).toContain('query="router middleware"'); + // Confirm the text payload is not valid JSON. + expect(() => JSON.parse(text)).toThrow(); + }); + + it("accepts format=text as an alias for text-v1", async () => { + const tool = createSearchTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + query: "router", + target: { registry: "npm", package_name: "express" }, + format: "text", + }, + {}, + ); + const text = result.content[0]?.text ?? ""; + expect(text).toContain("search | "); + }); + + it("keeps the JSON envelope when format=json (explicit)", async () => { + const tool = createSearchTool(createMockCodeNavigationService()); + const result = await tool.handler( + { + query: "router", + target: { registry: "npm", package_name: "express" }, + format: "json", + }, + {}, + ); + const payload = JSON.parse(result.content[0]?.text ?? "{}"); + expect(payload.completed).toBe(true); + }); }); diff --git a/src/tools/search.ts b/src/tools/search.ts index 70c0af31..7a0ede98 100644 --- a/src/tools/search.ts +++ b/src/tools/search.ts @@ -4,6 +4,8 @@ import { buildUnifiedSearchErrorPayload, buildUnifiedSearchParams, buildUnifiedSearchSuccessPayload, + renderUnifiedSearchError, + renderUnifiedSearchSuccess, toFileIntent, toSymbolCategory, toSymbolKind, @@ -72,6 +74,7 @@ export interface SearchArgs { limit?: number; offset?: number; wait_timeout_ms?: number; + format?: "json" | "text" | "text-v1"; } const schema = { @@ -150,6 +153,12 @@ const schema = { limit: z.coerce.number().int().min(1).max(100).optional(), offset: z.coerce.number().int().min(0).optional(), wait_timeout_ms: z.coerce.number().int().min(0).max(60000).optional(), + format: z + .enum(["json", "text", "text-v1"]) + .optional() + .describe( + 'Response format. Default `text-v1` — compact line-oriented output tuned for agent context efficiency. Pass `format: "json"` for the structured envelope (programmatic consumers, parity testing). `text` is an alias for `text-v1`. The text format is a public, snapshot-tested contract.', + ), }; const DESCRIPTION = @@ -217,11 +226,16 @@ export function createSearchTool( built.compiledQuery, outcome, ); + if (isTextFormat(args.format)) { + return textResult(renderUnifiedSearchSuccess(payload)); + } return textResult(JSON.stringify(payload)); } catch (error) { - return errorResult( - JSON.stringify(buildUnifiedSearchErrorPayload(error)), - ); + const payload = buildUnifiedSearchErrorPayload(error); + if (isTextFormat(args.format)) { + return errorResult(renderUnifiedSearchError(payload)); + } + return errorResult(JSON.stringify(payload)); } }, }; @@ -232,3 +246,12 @@ function isResolvedCodeTarget( ): target is ResolvedCodeTarget { return !("content" in target); } + +/** + * Default response format is text-v1 — agents consume the MCP surface + * and benefit from the compact form. Programmatic / parity callers + * opt into JSON explicitly. + */ +function isTextFormat(format: SearchArgs["format"]): boolean { + return format === undefined || format === "text" || format === "text-v1"; +} From 6be6f22e662e12a597fcdbf9c37f6d3e6ce190df Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Tue, 28 Apr 2026 12:30:15 +0300 Subject: [PATCH 2/2] docs(tools): correct code_read envelope and hint suppression rules Codex review of #42 flagged two stale claims in docs/implementation/tools.md: - The envelope listing for code_read omitted the optional hint? field. - The span-cap section claimed the hint appears whenever clamping fires, which contradicts the shipped suppression logic (no-op cap on small files, returned range reaching EOF). Update the envelope schema to include hint?, and rewrite the cap section to describe when the hint is actually emitted vs. suppressed, matching shouldEmitCappedHint in src/tools/read-file.ts. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/implementation/tools.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/implementation/tools.md b/docs/implementation/tools.md index d19c0911..fa4f38c9 100644 --- a/docs/implementation/tools.md +++ b/docs/implementation/tools.md @@ -110,7 +110,7 @@ These three indexing-gated tools share an addressing and lifecycle contract (doc **`code_files` envelope**: `{registry?|repoUrl?+gitRef?, total, hasMore, indexedVersion?, resolution?, files: [{path, name?, language?, fileType?, byteSize?}], hint?, filter?}`. `fileType` values preserve the service vocabulary (`CONFIG`, `SOURCE`, `DOC`, `TEST`). `total` is capped at returned count when `hasMore: true` — the terminal formatter renders `N+ files` in that case to avoid misleading users. `filter.pathPrefix` / `filter.limit` echo only when the caller supplied them explicitly; default limit (200) never round-trips. -**`code_read` envelope**: `{registry?|repoUrl?+gitRef?, path, language?, totalLines?, startLine?, endLine?, content?, isBinary?}`. `path` (not `filePath`) so the key matches `code_files.files[].path` and `code_grep.filter.path` when exact-file grep is used. Binary files set `isBinary: true` and **omit** `content` (not `null`); agents branch on the flag. +**`code_read` envelope**: `{registry?|repoUrl?+gitRef?, path, language?, totalLines?, startLine?, endLine?, content?, isBinary?, hint?}`. `path` (not `filePath`) so the key matches `code_files.files[].path` and `code_grep.filter.path` when exact-file grep is used. Binary files set `isBinary: true` and **omit** `content` (not `null`); agents branch on the flag. `hint` is emitted only when the MCP span cap actually truncated the response — see "code_read span cap" below. **`code_grep` envelope**: `{registry?|name?|repoUrl?+gitRef?, pattern, patternType?, caseSensitive?, matches: [{filePath, line, matchStartByte, matchEndByte, lineContent, contextBefore?, contextAfter?, fileContentHash?, fileIntent?, symbol?}], nextCursor?, hasMore, truncatedReason?, filesScanned, filesInScope, binaryFilesSkipped?, filesTooLargeSkipped?, totalMatches, uniqueFilesMatched, indexedVersion?, resolution?, filter?}`. Default-valued fields (`patternType: literal`, `caseSensitive: false`, zero skipped counters, `truncatedReason: none`) are omitted. `filter` echoes only explicit caller filters. Match entries carry `filePath` so grep output chains directly into `code_read`. @@ -137,7 +137,9 @@ All three code-navigation tools share the same indexing-retry contract. The stat **`FILE_NOT_FOUND` vs `NOT_FOUND`**: `code_read` / `code_grep` can hit "path doesn't resolve" errors when an exact path scope is invalid. The classifier is pre-wired to emit `FILE_NOT_FOUND` when the backend sends `extensions.code: "FILE_NOT_FOUND"`, but today the backend emits generic `NOT_FOUND` for both "package missing" and "path missing". The distinction is filed upstream. CLI terminal output for `code read` / `code grep` emits the hint "Use `code files` to list available paths." on both codes so users have an actionable next step regardless of classification. -**`code_read` span cap (MCP-only)**: real session traces showed agents requesting 300-600 line windows (and occasional unbounded full-file reads) which dominated context cost. The MCP surface caps each `code_read` call at `MCP_READ_MAX_SPAN` (150 lines, defined in `src/tools/read-file.ts`). The cap is enforced *before* the backend call — `deriveBoundedRange` clamps the request, so the service does not transfer bytes that will be discarded. When clamping fires, the envelope carries a `hint` field describing what was returned, what the caller originally requested, and the 80-150-line guidance for a follow-up. Binary files skip the hint. The CLI command `githits code read` does not apply the cap; humans piping whole files to disk continue to work. +**`code_read` span cap (MCP-only)**: real session traces showed agents requesting 300-600 line windows (and occasional unbounded full-file reads) which dominated context cost. The MCP surface caps each `code_read` call at `MCP_READ_MAX_SPAN` (150 lines, defined in `src/tools/read-file.ts`). The cap is enforced *before* the backend call — `deriveBoundedRange` clamps the request, so the service does not transfer bytes that will be discarded. + +The `hint` field is emitted only when the cap *actually truncated* the response — i.e., the returned range comes up short of available content. `shouldEmitCappedHint` (in `src/tools/read-file.ts`) suppresses the hint in three cases the agent doesn't need it: (a) the cap clamp didn't fire (caller's range was already within the cap); (b) the file fits within the cap, so the response is the whole file even though the request was clamped; (c) the returned range reaches end of file. Binary files always skip the hint. When emitted, the hint reads from `payload.startLine` / `endLine` / `totalLines` (the actual returned range, not the pre-clamp request) and includes the original request for the agent to learn from. The CLI command `githits code read` does not apply the cap; humans piping whole files to disk continue to work. ## Text response format (`format: "text-v1"`)