From eefcb5553494586d0807ceec6615533cff7acff2 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Wed, 6 May 2026 14:52:04 +0300 Subject: [PATCH 1/4] refactor: polish MCP instructions and tool descriptions Open-beta polish pass on agent-facing copy. The MCP server-level instructions now lead with a 4-bucket decision tree replacing the previous dense trigger-criteria paragraph, and the package/code-tools bullets are reordered to match how an agent navigates from a stack trace: search -> code reading (grep, read, files) -> docs -> package metadata. Per-tool descriptions normalized for consistency. The `format` parameter `.describe()` shares a base sentence across the 13 tools with tool-specific exceptions preserved verbatim (range semantics on `docs_read`, errors-stay-JSON on `code_files`/`code_grep`, `solution_id` contract on `get_example`, snapshot-tested-contract note on `search`). The `feedback` tool references `get_example` explicitly; `code_grep` acknowledges regex support; `code_read` allows direct-path use (e.g. from a stack trace); `search_status` bridges the camelCase `searchRef` response field to the snake_case `search_ref` parameter. Registry coverage strings now derive from canonical `PKGSEER_REGISTRY_ARGS` order. The hand-rolled `SUPPORTED_DEPS_REGISTRIES_HUMAN` constant is replaced with `SUPPORTED_DEPS_REGISTRIES_LIST` filtered from the source of truth, eliminating an order-drift class. The `pkg_info`, `pkg_deps`, and `pkg_changelog` descriptions and the CLI `pkg deps --help` align to the canonical proper-cased order. The instructions test suite gains structural invariants for decision- tree presence, bullet ordering, and tip placement; the mention to registration test stays strict. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/commands/mcp-instructions.test.ts | 65 +++++++++++ src/commands/mcp-instructions.ts | 120 ++++++++++++--------- src/commands/pkg/deps.test.ts | 2 +- src/commands/pkg/deps.ts | 8 +- src/shared/package-dependencies-request.ts | 22 ++-- src/tools/feedback.ts | 6 +- src/tools/get-example.ts | 2 +- src/tools/grep-repo.test.ts | 2 +- src/tools/grep-repo.ts | 8 +- src/tools/list-files.ts | 24 ++--- src/tools/package-changelog.ts | 16 +-- src/tools/package-dependencies.test.ts | 11 +- src/tools/package-dependencies.ts | 27 +++-- src/tools/package-summary.test.ts | 6 +- src/tools/package-summary.ts | 10 +- src/tools/package-vulnerabilities.test.ts | 6 +- src/tools/package-vulnerabilities.ts | 14 +-- src/tools/search-language.ts | 2 +- src/tools/search-status.ts | 8 +- src/tools/search.ts | 7 +- 20 files changed, 229 insertions(+), 137 deletions(-) diff --git a/src/commands/mcp-instructions.test.ts b/src/commands/mcp-instructions.test.ts index cc5d3bcb..2dc34f79 100644 --- a/src/commands/mcp-instructions.test.ts +++ b/src/commands/mcp-instructions.test.ts @@ -127,4 +127,69 @@ describe("buildMcpInstructions", () => { expect(mentioned.has(name)).toBe(true); } }); + + it("ships a decision tree mentioning all three workflow tools in the core block", () => { + const deps = createTestDeps(); + const instructions = buildMcpInstructions(deps); + const coreEnd = instructions.indexOf("Indexed package/source tools"); + const coreSection = instructions.slice(0, coreEnd); + + expect(coreSection).toContain("`get_example`"); + expect(coreSection).toContain("`search`"); + expect(coreSection).toContain("`feedback`"); + expect(coreSection).toContain("`search_language`"); + }); + + it("orders package-section bullets by agent decision flow", () => { + const deps = createTestDeps(); + const instructions = buildMcpInstructions(deps); + + const positions = { + search: instructions.indexOf("- `search` —"), + searchStatus: instructions.indexOf("- `search_status`"), + codeGrep: instructions.indexOf("- `code_grep`"), + codeRead: instructions.indexOf("- `code_read`"), + codeFiles: instructions.indexOf("- `code_files`"), + docsList: instructions.indexOf("- `docs_list`"), + docsRead: instructions.indexOf("- `docs_read`"), + pkgInfo: instructions.indexOf("- `pkg_info`"), + pkgVulns: instructions.indexOf("- `pkg_vulns`"), + pkgDeps: instructions.indexOf("- `pkg_deps`"), + pkgChangelog: instructions.indexOf("- `pkg_changelog`"), + }; + + for (const [name, idx] of Object.entries(positions)) { + expect(idx).toBeGreaterThan(-1); + expect(`${name}=${idx}`).not.toContain("=-1"); + } + + // Discovery first, then code reading (grep → read → files), then + // docs, then package metadata. `code_files` follows `code_read` + // because it is a path-discovery fallback, not the step after a hit. + expect(positions.search).toBeLessThan(positions.searchStatus); + expect(positions.searchStatus).toBeLessThan(positions.codeGrep); + expect(positions.codeGrep).toBeLessThan(positions.codeRead); + expect(positions.codeRead).toBeLessThan(positions.codeFiles); + expect(positions.codeFiles).toBeLessThan(positions.docsList); + expect(positions.docsList).toBeLessThan(positions.docsRead); + expect(positions.docsRead).toBeLessThan(positions.pkgInfo); + expect(positions.pkgInfo).toBeLessThan(positions.pkgVulns); + expect(positions.pkgVulns).toBeLessThan(positions.pkgDeps); + expect(positions.pkgDeps).toBeLessThan(positions.pkgChangelog); + }); + + it("places the strategy tip after the bullets and the delegation tip before them", () => { + const deps = createTestDeps(); + const instructions = buildMcpInstructions(deps); + + const delegationIdx = instructions.indexOf( + "Delegate multi-call work to a sub-agent", + ); + const firstBulletIdx = instructions.indexOf("- `search` —"); + const lastBulletIdx = instructions.indexOf("- `pkg_changelog`"); + const strategyIdx = instructions.indexOf("Strategy — reference-first"); + + expect(delegationIdx).toBeLessThan(firstBulletIdx); + expect(strategyIdx).toBeGreaterThan(lastBulletIdx); + }); }); diff --git a/src/commands/mcp-instructions.ts b/src/commands/mcp-instructions.ts index a62c4f14..7cc90f05 100644 --- a/src/commands/mcp-instructions.ts +++ b/src/commands/mcp-instructions.ts @@ -13,16 +13,36 @@ import type { Dependencies } from "../container.js"; * guidance for the same always-on tool surface the server registers. */ -const CORE_BLOCK = `GitHits provides verified, canonical code examples from global open source. Use it for global solution synthesis and canonical examples when you're stuck, repeated attempts failed, 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. +const CORE_BLOCK = `GitHits provides verified, canonical code examples from global open source for AI agents. Pick a path: -Example workflow: call \`get_example\` with one focused question; pass \`language\` only when you know the exact language name, otherwise call \`search_language\` first. Default output is markdown with a trailing \`solution_id\`; send \`feedback\` on that solution_id. Reuse prior results before searching again. For dependency-specific grounding, use package-scoped \`search\` before global \`get_example\`.`; +- Need a canonical example or cross-project pattern with no specific package pinned (you're stuck, repeated attempts failed, the user wants up-to-date API usage, or the user mentioned GitHits) — call \`get_example\` for global solution synthesis. +- Inspecting a specific known dependency or repository (stack trace points there, verifying how a particular library works, evaluating an upgrade) — call \`search\` plus the indexed code, docs, and package tools listed below. +- Question is comparative across OSS projects (e.g. "how does X vs Y handle Z") or requires reading how a real codebase implements a feature — combine the two: \`search\` for known targets, \`get_example\` for cross-project synthesis. +- Rate a result so it improves future answers — call \`feedback\` with the \`solution_id\` from a prior \`get_example\` response. + +\`get_example\` workflow: pass \`language\` only when you know the exact name; otherwise call \`search_language\` first. Default output is markdown with a trailing \`solution_id\`. Reuse prior results before searching again. For dependency-specific grounding, prefer package-scoped \`search\` before global \`get_example\`.`; const PACKAGE_TOOLS_PREAMBLE = `Indexed package/source tools inspect third-party dependency source, docs, and registry metadata. Use them when a stack trace points into a dependency, you need to verify how a library works, or you're evaluating whether to add or upgrade a package. Package spec: \`registry:name[@version]\`. Default outputs are compact \`text-v1\` for agent context efficiency; pass \`format: "json"\` only when you need structured fields for programmatic parsing.`; -const PKG_INFO_BULLET = - '- `pkg_info` — compact package overview: latest version, license, downloads, quickstart, and active advisory count. Pass `format: "json"` for the structured envelope.'; +const MULTI_TURN_TIP = + '**Delegate multi-call work to a sub-agent.** Code-navigation work (`search`, `code_grep`, `code_read`, `code_files`) often takes 3-10 calls. For cross-project comparisons, codebase mapping, pattern surveys, or "how does X actually work" investigations, spawn a sub-task/sub-agent and ask for a compact synthesis. Do it inline only when the raw snippet belongs in the main conversation.'; + +const SEARCH_BULLET = + '- `search` — unified search across indexed dependency code, docs, and symbols. Omit `sources` for AUTO. Use `sources:["code"]` for implementation/examples/tests text, `sources:["symbol"]` for precise API/entity lookup, and `sources:["docs"]` for guides/reference/changelogs. Default output is compact `text-v1` with ready-to-call follow-up arguments; pass `format: "json"` for structured locators, highlights, and source status. Complete by default; opt into partial hits with `allow_partial_results: true`. Incomplete responses carry a `searchRef`.'; + +const SEARCH_STATUS_BULLET = + "- `search_status` — follow up a prior `search` by `searchRef` to check progress, fetch partial hits, or fetch final results."; + +const CODE_GREP_BULLET = + "- `code_grep` — deterministic text or regex grep over indexed source. Use it when you know the pattern; use `search` for discovery. Narrow with `path`, `path_prefix`, `globs`, or `extensions`. Each match's `path:line` chains into `code_read`."; + +const CODE_READ_BULLET = + "- `code_read` — read a dependency file by `path`. **MCP cap: 150 lines per call**. When you already have an exact path (e.g. from a stack trace), call this directly; otherwise locate it first with `search` or `code_grep` and pick a focused `start_line` / `end_line` window. Binary files set `isBinary: true` and omit `content`. On `FILE_NOT_FOUND` / `NOT_FOUND`, call `code_files` for the actual path."; + +const CODE_FILES_BULLET = + '- `code_files` — list files in an indexed dependency. `path`, `path_prefix`, and `globs` are OR-ed selectors; extensions, language, file type, and file-intent filters intersect on top. Returned paths feed into `code_read` and help scope `code_grep`; pass `format: "json"` for language/type/size metadata.'; const DOCS_LIST_BULLET = "- `docs_list` — browse hosted and repository-backed package docs. Entries include stable pageIds, source URLs, and repo-file follow-up metadata when available."; @@ -30,6 +50,9 @@ const DOCS_LIST_BULLET = const DOCS_READ_BULLET = "- `docs_read` — read a documentation page by pageId. Works for both hosted docs and repo-backed docs."; +const PKG_INFO_BULLET = + '- `pkg_info` — compact package overview: latest version, license, downloads, quickstart, and active advisory count. Pass `format: "json"` for the structured envelope.'; + const PKG_VULNS_BULLET = '- `pkg_vulns` — compact known CVE / OSV advisory summary for npm, PyPI, Hex, or Crates packages, optionally pinned to `version`. Filter with `min_severity`; include retracted advisories with `include_withdrawn`. Pass `format: "json"` for per-advisory structured fields.'; @@ -39,29 +62,16 @@ const PKG_DEPS_BULLET = const PKG_CHANGELOG_BULLET = '- `pkg_changelog` — compact release notes for a package or GitHub repo, newest-first. Default latest mode returns recent entries with markdown body previews; `from_version` switches to range mode. Set `include_bodies: false` for a compact timeline or pass `format: "json"` for full bodies.'; -const SEARCH_BULLET = - '- `search` — unified search across indexed dependency code, docs, and symbols. Omit `sources` for AUTO. Use `sources:["code"]` for implementation/examples/tests text, `sources:["symbol"]` for precise API/entity lookup, and `sources:["docs"]` for guides/reference/changelogs. Default output is compact `text-v1` with ready-to-call follow-up arguments; pass `format: "json"` for structured locators, highlights, and source status. Complete by default; opt into partial hits with `allow_partial_results: true`. Incomplete responses carry a `searchRef`.'; - -const SEARCH_STATUS_BULLET = - "- `search_status` — follow up a prior `search` by `searchRef` to check progress, fetch partial hits, or fetch final results."; - -const CODE_FILES_BULLET = - '- `code_files` — list files in an indexed dependency. `path`, `path_prefix`, and `globs` are OR-ed selectors; extensions, language, file type, and file-intent filters intersect on top. Returned paths feed into `code_read` and help scope `code_grep`; pass `format: "json"` for language/type/size metadata.'; - -const CODE_READ_BULLET = - "- `code_read` — read a dependency file by `path`. **MCP cap: 150 lines per call**; choose focused `start_line` / `end_line` windows from `search` or `code_grep`. Binary files set `isBinary: true` and omit `content`. On `FILE_NOT_FOUND` / `NOT_FOUND`, call `code_files` for the actual path."; - -const CODE_GREP_BULLET = - "- `code_grep` — deterministic text or regex grep over indexed source. Use it when you know the pattern; use `search` for discovery. Narrow with `path`, `path_prefix`, `globs`, or `extensions`. Each match's `path:line` chains into `code_read`."; - -const SEARCH_VS_SYMBOLS_TIP = - 'Use code-first grounding for behavioral claims: source, symbols, tests, examples, and call sites beat docs prose. Prefer unified `search` for indexed dependency and repository discovery; use `sources:["symbol"]` when you want symbol-shaped results. Prefer `get_example` for global canonical example retrieval after package-scoped grounding. 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 needed window with `code_read` using explicit `start_line` / `end_line` values, typically 80-150 lines around the match. The MCP `code_read` surface caps each call at 150 lines."; - -const MULTI_TURN_TIP = - '**Delegate multi-call work to a sub-agent.** Code-navigation work (`search`, `code_grep`, `code_read`, `code_files`) often takes 3-10 calls. For cross-project comparisons, codebase mapping, pattern surveys, or "how does X actually work" investigations, spawn a sub-task/sub-agent and ask for a compact synthesis. Do it inline only when the raw snippet belongs in the main conversation.'; +/** + * Combined strategy tip. Replaces the earlier + * `REFERENCE_FIRST_TIP` + `SEARCH_VS_SYMBOLS_TIP` pair, which + * overlapped: both told agents to grep-then-read and both + * contrasted `search`/`code_grep`/`get_example`. Phrase + * "reference-first" stays in the section name so prior agent + * habits and the test invariant continue to anchor here. + */ +const STRATEGY_TIP = + 'Strategy — reference-first. Locate symbols and lines with `search` or `code_grep` first, then read the needed window with `code_read` using explicit `start_line` / `end_line` (typical 80-150 lines around the match; the MCP `code_read` surface caps each call at 150 lines). Source, symbols, tests, and call sites beat docs prose for behavioral claims; use `sources:["symbol"]` when you want symbol-shaped results, `code_grep` for deterministic text matching, and `get_example` for global canonical examples after package-scoped grounding.'; /** * Build the server-level instructions string for the current session. @@ -71,31 +81,35 @@ const MULTI_TURN_TIP = * with the registered tool surface. */ export function buildMcpInstructions(_deps: Dependencies): string { - const sections = [CORE_BLOCK]; - - const bullets: string[] = []; - bullets.push(DOCS_LIST_BULLET); - bullets.push(DOCS_READ_BULLET); - bullets.push(PKG_INFO_BULLET); - bullets.push(PKG_VULNS_BULLET); - bullets.push(PKG_DEPS_BULLET); - bullets.push(PKG_CHANGELOG_BULLET); - bullets.push(SEARCH_BULLET); - bullets.push(SEARCH_STATUS_BULLET); - bullets.push(CODE_FILES_BULLET); - bullets.push(CODE_READ_BULLET); - bullets.push(CODE_GREP_BULLET); - - const parts = [PACKAGE_TOOLS_PREAMBLE]; + // Bullets ordered by agent decision flow: discovery (search) → + // code reading (grep, read, files) → docs → package metadata. + // `code_files` follows `code_read` because it is the path- + // discovery fallback when a known path doesn't resolve, not the + // step after a hit. Each bullet name↔registration is enforced by + // `mcp-instructions.test.ts`. + const bullets = [ + SEARCH_BULLET, + SEARCH_STATUS_BULLET, + CODE_GREP_BULLET, + CODE_READ_BULLET, + CODE_FILES_BULLET, + DOCS_LIST_BULLET, + DOCS_READ_BULLET, + PKG_INFO_BULLET, + PKG_VULNS_BULLET, + PKG_DEPS_BULLET, + PKG_CHANGELOG_BULLET, + ]; + // Lead with delegation because it is the highest-leverage decision - // for code-navigation work. The reference-first / decision tips - // follow the bullets where they act as workflow guidance for the - // chosen tool. - parts.push(MULTI_TURN_TIP); - parts.push(bullets.join("\n")); - parts.push(REFERENCE_FIRST_TIP); - parts.push(SEARCH_VS_SYMBOLS_TIP); - - sections.push(parts.join("\n\n")); - return sections.join("\n\n"); + // for code-navigation work. The strategy tip follows the bullets + // where it acts as workflow guidance for the chosen tool. + const packageSection = [ + PACKAGE_TOOLS_PREAMBLE, + MULTI_TURN_TIP, + bullets.join("\n"), + STRATEGY_TIP, + ].join("\n\n"); + + return [CORE_BLOCK, packageSection].join("\n\n"); } diff --git a/src/commands/pkg/deps.test.ts b/src/commands/pkg/deps.test.ts index 002abd31..547c5a05 100644 --- a/src/commands/pkg/deps.test.ts +++ b/src/commands/pkg/deps.test.ts @@ -216,7 +216,7 @@ describe("pkgDepsAction", () => { } expect(errorSpy.mock.calls[0]?.[0]).toBe( - "pkg deps only supports npm, pypi, hex, crates, vcpkg, zig, rubygems, and go. Got: nuget.", + "pkg deps only supports npm, pypi, hex, crates, zig, vcpkg, rubygems, go. Got: nuget.", ); errorSpy.mockRestore(); exitSpy.mockRestore(); diff --git a/src/commands/pkg/deps.ts b/src/commands/pkg/deps.ts index d5dc8678..54394728 100644 --- a/src/commands/pkg/deps.ts +++ b/src/commands/pkg/deps.ts @@ -10,7 +10,10 @@ import { parsePackageSpec, requireAuth, } from "../../shared/index.js"; -import { buildPackageDependenciesParams } from "../../shared/package-dependencies-request.js"; +import { + buildPackageDependenciesParams, + SUPPORTED_DEPS_REGISTRIES_LIST, +} from "../../shared/package-dependencies-request.js"; import { buildPackageDependenciesSuccessPayload, formatPackageDependenciesTerminal, @@ -198,8 +201,7 @@ groups). Concrete --lifecycle values include runtime plus matching groups. conflict detection, and circular-dependency flags. Package spec: :[@]. Supported registries: -npm, pypi, hex, crates, vcpkg, zig, rubygems, go. Omit @ for the latest -release.`; +${SUPPORTED_DEPS_REGISTRIES_LIST}. Omit @ for the latest release.`; export function registerPkgDepsCommand(pkgCommand: Command): Command { return pkgCommand diff --git a/src/shared/package-dependencies-request.ts b/src/shared/package-dependencies-request.ts index a68496da..0245a8f7 100644 --- a/src/shared/package-dependencies-request.ts +++ b/src/shared/package-dependencies-request.ts @@ -7,10 +7,10 @@ * Responsibilities: * - Trim + validate `packageName`. * - Normalise registry case and restrict to the registries that the - * upstream `packageDependencies` resolver supports (`npm, pypi, hex, - * crates, vcpkg, zig`). Other known registries are rejected with a - * tool-specific message; truly unknown registries fall through to - * the shared `UnsupportedRegistryError`. + * upstream `packageDependencies` resolver supports (see + * `SUPPORTED_DEPS_REGISTRIES`). Other known registries are rejected + * with a tool-specific message; truly unknown registries fall + * through to the shared `UnsupportedRegistryError`. * - Reject tag-style versions (`v4.18.0`) client-side — the `v` prefix * is a git-tag convention, not a canonical version on any supported * registry. @@ -26,6 +26,7 @@ import { } from "./package-spec.js"; import { isKnownPkgseerRegistryArg, + PKGSEER_REGISTRY_ARGS, PKGSEER_REGISTRY_LIST, type PkgseerRegistry, type PkgseerRegistryArg, @@ -81,8 +82,15 @@ export const SUPPORTED_DEPS_REGISTRIES: ReadonlySet = new Set([ "GO", ]); -export const SUPPORTED_DEPS_REGISTRIES_HUMAN = - "npm, pypi, hex, crates, vcpkg, zig, rubygems, and go"; +/** + * Lowercase deps-supported registries, comma-separated, in the + * canonical order defined by `PKGSEER_REGISTRY_ARGS`. Derived rather + * than hand-rolled so the order propagates from the single source of + * truth and a future registry addition shows up here automatically. + */ +export const SUPPORTED_DEPS_REGISTRIES_LIST = PKGSEER_REGISTRY_ARGS.filter( + (arg) => SUPPORTED_DEPS_REGISTRIES.has(toPkgseerRegistry(arg)), +).join(", "); export function supportsDependenciesRegistry( registry: PkgseerRegistry, @@ -141,7 +149,7 @@ export function buildPackageDependenciesParams( ); if (!supportsDependenciesRegistry(registry)) { throw new UnsupportedDependenciesRegistryError( - `pkg deps only supports ${SUPPORTED_DEPS_REGISTRIES_HUMAN}. Got: ${normalisedRegistryArg}.`, + `pkg deps only supports ${SUPPORTED_DEPS_REGISTRIES_LIST}. Got: ${normalisedRegistryArg}.`, ); } diff --git a/src/tools/feedback.ts b/src/tools/feedback.ts index 198e06f9..dbb3d618 100644 --- a/src/tools/feedback.ts +++ b/src/tools/feedback.ts @@ -14,7 +14,7 @@ const schema = { .string() .min(1) .describe( - "The solution ID from a previous search result (shown in the result)", + "The `solution_id` returned by a prior `get_example` call (shown on the trailing line of the markdown result, or under the `solution_id` key in JSON mode).", ), accepted: z .boolean() @@ -27,9 +27,9 @@ const schema = { ), }; -const DESCRIPTION = `Submit feedback on a GitHits example result. +const DESCRIPTION = `Submit feedback on a \`get_example\` result. -Call after \`get_example\` to record whether the returned example was used. \`accepted=true\` when it solved the problem or was useful; \`accepted=false\` when it was irrelevant or wrong. Use \`feedback_text\` to add a short reason. Feeds back into ranking quality.`; +Call after \`get_example\` with the returned \`solution_id\`. \`accepted=true\` when the example solved the problem or was useful; \`accepted=false\` when it was irrelevant or wrong. Use \`feedback_text\` to add a short reason. Feeds ranking quality. Not for unified \`search\` hits — those have no \`solution_id\`.`; export function createFeedbackTool( service: GitHitsService, diff --git a/src/tools/get-example.ts b/src/tools/get-example.ts index 5265f7ce..c2b74030 100644 --- a/src/tools/get-example.ts +++ b/src/tools/get-example.ts @@ -33,7 +33,7 @@ const schema = { .enum(["json", "text", "text-v1"]) .optional() .describe( - "Response format. Default `text-v1` returns markdown directly with a trailing `solution_id` line when available. Pass `json` for `{result, solution_id?}`.", + 'Response format. Default `text-v1` returns markdown directly with a trailing `solution_id` line when available. Pass `format: "json"` for `{result, solution_id?}`.', ), }; diff --git a/src/tools/grep-repo.test.ts b/src/tools/grep-repo.test.ts index 4d250cc5..a5743bd9 100644 --- a/src/tools/grep-repo.test.ts +++ b/src/tools/grep-repo.test.ts @@ -17,7 +17,7 @@ describe("createGrepRepoTool — metadata", () => { it("registers the correct tool name and schema keys", () => { const tool = createGrepRepoTool(createMockCodeNavigationService()); expect(tool.name).toBe("code_grep"); - expect(tool.description).toContain("Deterministic text grep"); + expect(tool.description).toContain("Deterministic text or regex grep"); expect(Object.keys(tool.schema).sort()).toEqual([ "case_sensitive", "context_lines", diff --git a/src/tools/grep-repo.ts b/src/tools/grep-repo.ts index 70388b51..948c2752 100644 --- a/src/tools/grep-repo.ts +++ b/src/tools/grep-repo.ts @@ -89,11 +89,11 @@ const schema = { }; 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. " + + "Deterministic text or regex grep over indexed dependency and repository source files. " + + 'Use this when you know the pattern (literal by default; pass `pattern_type: "regex"` for RE2). ' + + "Use `search` for discovery instead. " + "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`)."; + "Each match's `path` chains into `code_read.path`; pick a window around `match.line` for `code_read.start_line` / `end_line`."; export function createGrepRepoTool( service: CodeNavigationService, diff --git a/src/tools/list-files.ts b/src/tools/list-files.ts index 6d7fb3fb..951100bf 100644 --- a/src/tools/list-files.ts +++ b/src/tools/list-files.ts @@ -103,23 +103,23 @@ const schema = { .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.', + 'Response format. Default `text-v1` — compact paths-only listing. 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. 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. Narrow with `path`, `path_prefix`, `globs`, " + + "List files in an indexed dependency. Use this to discover paths " + + "before `code_read` (when `code_read` returns `FILE_NOT_FOUND` or " + + "you don't yet know the path) and to scope `code_grep`. Address " + + "via `target.registry` + `target.package_name` (package scope) or " + + "`target.repo_url` + `target.git_ref` (repo scope), mutually " + + "exclusive. Narrow with `path`, `path_prefix`, `globs`, " + "`extensions`, `file_types`, `languages`, or file-intent 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`."; + "JSON envelope shape: `{total, hasMore, files: [{path, name, " + + "language, fileType, byteSize}], resolution, indexedVersion}`. " + + "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, diff --git a/src/tools/package-changelog.ts b/src/tools/package-changelog.ts index 9fa2c66e..76406841 100644 --- a/src/tools/package-changelog.ts +++ b/src/tools/package-changelog.ts @@ -92,7 +92,7 @@ const schema = { .enum(["json", "text", "text-v1"]) .optional() .describe( - "Response format. Default `text-v1` is compact for agents. Pass `json` for the structured envelope.", + 'Response format. Default `text-v1` — compact entry timeline with body previews. Pass `format: "json"` for the structured envelope with full markdown bodies.', ), }; @@ -104,13 +104,13 @@ const DESCRIPTION = "Address via `registry` + `package_name` or `repo_url` (mutually " + 'exclusive). Response includes optional `source` (`"releases"` / ' + '`"changelog_file"` / `"hexdocs"`) when a concrete changelog source ' + - 'exists, `mode` (`"latest"` or `"range"`), ' + - 'and entries with markdown body previews. Pass `format: "json"` ' + - "for the structured envelope with full markdown bodies. Set " + - "`include_bodies: false` for a version / date / URL timeline only. " + - "Package-version entries without changelog text succeed with `source` " + - "omitted; no-source plus no entries returns `NOT_FOUND`. Supports npm, " + - "PyPI, Hex, Crates, vcpkg, Zig, NuGet, Maven, Packagist, RubyGems, Go."; + 'exists, `mode` (`"latest"` or `"range"`), and entries with ' + + "markdown body previews. Set `include_bodies: false` for a " + + 'version / date / URL timeline only; pass `format: "json"` for ' + + "full markdown bodies. Package-version entries without changelog " + + "text succeed with `source` omitted; no-source plus no entries " + + "returns `NOT_FOUND`. Supports npm, PyPI, Hex, Crates, NuGet, " + + "Maven, Zig, vcpkg, Packagist, RubyGems, and Go."; export function createPackageChangelogTool( service: PackageIntelligenceService, diff --git a/src/tools/package-dependencies.test.ts b/src/tools/package-dependencies.test.ts index 89180315..224aa443 100644 --- a/src/tools/package-dependencies.test.ts +++ b/src/tools/package-dependencies.test.ts @@ -16,9 +16,12 @@ describe("createPackageDependenciesTool — metadata", () => { createMockPackageIntelligenceService(), ); expect(tool.name).toBe("pkg_deps"); - expect(tool.description).toContain("npm, PyPI, Hex, Crates"); - expect(tool.description).toContain("Default output is compact"); - expect(tool.description).toContain('format: "json"'); + // Canonical registry order from PKGSEER_REGISTRY_ARGS, restricted + // to the deps-supported subset. + expect(tool.description).toContain( + "npm, PyPI, Hex, Crates, Zig, vcpkg, RubyGems, and Go", + ); + expect(tool.description).toContain("dependency graph"); expect(Object.keys(tool.schema).sort()).toEqual([ "format", "include_importers", @@ -292,7 +295,7 @@ describe("createPackageDependenciesTool — validation errors via in-handler bui const payload = parseText(result) as { code: string; error: string }; expect(payload.code).toBe("INVALID_ARGUMENT"); expect(payload.error).toBe( - "pkg deps only supports npm, pypi, hex, crates, vcpkg, zig, rubygems, and go. Got: nuget.", + "pkg deps only supports npm, pypi, hex, crates, zig, vcpkg, rubygems, go. Got: nuget.", ); }); diff --git a/src/tools/package-dependencies.ts b/src/tools/package-dependencies.ts index 167f7fd1..df242664 100644 --- a/src/tools/package-dependencies.ts +++ b/src/tools/package-dependencies.ts @@ -3,7 +3,7 @@ import type { PackageIntelligenceService } from "../services/index.js"; import { InvalidPackageSpecError } from "../shared/index.js"; import { buildPackageDependenciesParams, - SUPPORTED_DEPS_REGISTRIES_HUMAN, + SUPPORTED_DEPS_REGISTRIES_LIST, } from "../shared/package-dependencies-request.js"; import { buildPackageDependenciesSuccessPayload, @@ -36,7 +36,7 @@ const schema = { registry: z .string() .describe( - `Package registry. Dependency data is available on ${SUPPORTED_DEPS_REGISTRIES_HUMAN}.`, + `Package registry. Dependency data is available on ${SUPPORTED_DEPS_REGISTRIES_LIST}.`, ), package_name: z .string() @@ -78,22 +78,21 @@ const schema = { .enum(["json", "text", "text-v1"]) .optional() .describe( - "Response format. Default `text-v1` is compact for agents. Pass `json` for the structured envelope.", + 'Response format. Default `text-v1` — compact dependency listing. Pass `format: "json"` for the structured envelope.', ), }; const DESCRIPTION = - "Analyze a package's dependency graph. Default output is compact " + - "text listing direct runtime dependencies with resolved versions; " + - 'pass `format: "json"` for the structured envelope. Non-runtime ' + - "groups are omitted by default for token efficiency. Use `lifecycle` " + - "with a concrete value for runtime plus matching groups, or `all` " + - "for runtime plus all available groups. Set " + - "`include_transitive: true` to add a `transitive` block with the " + - "full install footprint, conflict detection, and circular-" + - "dependency flags; layer `include_importers: true` on top when you " + - "also need per-package provenance. Supports npm, PyPI, Hex, Crates, " + - "RubyGems, Go, vcpkg, and Zig."; + "Analyze a package's dependency graph. Lists direct runtime " + + "dependencies with resolved versions; non-runtime groups are " + + "omitted by default. Use `lifecycle` with a concrete value for " + + "runtime plus matching groups, or `all` for runtime plus every " + + "available group. Set `include_transitive: true` to add a " + + "`transitive` block with the full install footprint, conflict " + + "detection, and circular-dependency flags; layer " + + "`include_importers: true` on top when you also need per-package " + + "provenance. Supports npm, PyPI, Hex, Crates, Zig, vcpkg, RubyGems, " + + "and Go."; export function createPackageDependenciesTool( service: PackageIntelligenceService, diff --git a/src/tools/package-summary.test.ts b/src/tools/package-summary.test.ts index bcf2cb86..71c82696 100644 --- a/src/tools/package-summary.test.ts +++ b/src/tools/package-summary.test.ts @@ -17,8 +17,10 @@ describe("createPackageSummaryTool — metadata", () => { ); expect(tool.name).toBe("pkg_info"); expect(tool.description).toContain("package overview"); - expect(tool.description).toContain("Default output is compact text"); - expect(tool.description).toContain('format: "json"'); + // Canonical registry order from PKGSEER_REGISTRY_ARGS + expect(tool.description).toContain( + "npm, PyPI, Hex, Crates, NuGet, Maven, Zig, vcpkg, Packagist, RubyGems, and Go", + ); expect(Object.keys(tool.schema)).toEqual([ "registry", "package_name", diff --git a/src/tools/package-summary.ts b/src/tools/package-summary.ts index ab27c23c..1c2a384b 100644 --- a/src/tools/package-summary.ts +++ b/src/tools/package-summary.ts @@ -33,7 +33,7 @@ const schema = { .enum(["json", "text", "text-v1"]) .optional() .describe( - "Response format. Default `text-v1` is compact for agents. Pass `json` for the structured envelope.", + 'Response format. Default `text-v1` — compact package overview. Pass `format: "json"` for the structured envelope.', ), }; @@ -42,11 +42,9 @@ const DESCRIPTION = "repository, downloads, GitHub stars, install command, recent " + "changes, and a count of known vulnerabilities. Use before " + "recommending a package or to orient on what a dependency is. " + - 'Default output is compact text; pass `format: "json"` for the ' + - "structured envelope. " + - "Works across npm, PyPI, Hex, Crates, NuGet, Maven, Packagist, " + - "RubyGems, Go, vcpkg, and Zig. Always returns data for the latest published " + - "version."; + "Works across npm, PyPI, Hex, Crates, NuGet, Maven, Zig, vcpkg, " + + "Packagist, RubyGems, and Go. Always returns data for the latest " + + "published version."; export function createPackageSummaryTool( service: PackageIntelligenceService, diff --git a/src/tools/package-vulnerabilities.test.ts b/src/tools/package-vulnerabilities.test.ts index 09587d55..49a014c8 100644 --- a/src/tools/package-vulnerabilities.test.ts +++ b/src/tools/package-vulnerabilities.test.ts @@ -16,9 +16,9 @@ describe("createPackageVulnerabilitiesTool — metadata", () => { createMockPackageIntelligenceService(), ); expect(tool.name).toBe("pkg_vulns"); - expect(tool.description).toContain("npm, PyPI, Hex, or"); - expect(tool.description).toContain("Default output is compact text"); - expect(tool.description).toContain('format: "json"'); + // Capability subset is intentionally narrow for vulnerability data. + expect(tool.description).toContain("npm, PyPI, Hex, or Crates"); + expect(tool.description).toContain("known vulnerabilities"); expect(Object.keys(tool.schema).sort()).toEqual([ "format", "include_withdrawn", diff --git a/src/tools/package-vulnerabilities.ts b/src/tools/package-vulnerabilities.ts index 75b6e755..c1f17d54 100644 --- a/src/tools/package-vulnerabilities.ts +++ b/src/tools/package-vulnerabilities.ts @@ -50,7 +50,7 @@ const schema = { .enum(["json", "text", "text-v1"]) .optional() .describe( - "Response format. Default `text-v1` is compact for agents. Pass `json` for the structured envelope.", + 'Response format. Default `text-v1` — compact advisory summary. Pass `format: "json"` for the structured envelope.', ), }; @@ -58,12 +58,12 @@ const DESCRIPTION = "Check known vulnerabilities for a package on npm, PyPI, Hex, or " + "Crates (other registries are not yet supported for vulnerability " + "data). Returns a count summary, each advisory with OSV ID, " + - "severity, affected ranges, and fix versions. Malicious-package advisories surface in a separate " + - "bucket. Pass `version` to inspect a specific release; otherwise " + - "the latest is checked. Use `min_severity` to filter to a threshold " + - "(`low`, `medium`, `high`, `critical`) and `include_withdrawn` to " + - "also see retracted advisories. Default output is compact text; " + - 'pass `format: "json"` for the structured envelope.'; + "severity, affected ranges, and fix versions. Malicious-package " + + "advisories surface in a separate bucket. Pass `version` to inspect " + + "a specific release; otherwise the latest is checked. Use " + + "`min_severity` to filter to a threshold (`low`, `medium`, `high`, " + + "`critical`) and `include_withdrawn` to also see retracted " + + "advisories."; export function createPackageVulnerabilitiesTool( service: PackageIntelligenceService, diff --git a/src/tools/search-language.ts b/src/tools/search-language.ts index 52a875fa..297cc4c8 100644 --- a/src/tools/search-language.ts +++ b/src/tools/search-language.ts @@ -20,7 +20,7 @@ const schema = { .enum(["json", "text", "text-v1"]) .optional() .describe( - "Response format. Default `text-v1` returns one language per line. Pass `json` for the structured array.", + 'Response format. Default `text-v1` returns one language per line. Pass `format: "json"` for the structured array.', ), }; diff --git a/src/tools/search-status.ts b/src/tools/search-status.ts index 7e1b26ee..f1fc6f76 100644 --- a/src/tools/search-status.ts +++ b/src/tools/search-status.ts @@ -16,7 +16,9 @@ const schema = { search_ref: z .string() .min(1) - .describe("Search reference returned by search."), + .describe( + "The `searchRef` field from a prior `search` response (camelCase in the response, snake_case as this parameter). Pass it through unchanged.", + ), format: z .enum(["json", "text", "text-v1"]) .optional() @@ -26,8 +28,8 @@ const schema = { }; const DESCRIPTION = - "Check progress, fetch partial hits when the original request used allow_partial_results: true, or fetch final results for a prior unified search. " + - "Pass the search_ref returned by `search` when the original request did not complete within the wait window."; + "Check progress, fetch partial hits (when the original request used `allow_partial_results: true`), or fetch final results for a prior `search` that returned a `searchRef`. " + + "Pass the `searchRef` from that response as `search_ref` here (response field is camelCase; this parameter is snake_case)."; export function createSearchStatusTool( service: CodeNavigationService, diff --git a/src/tools/search.ts b/src/tools/search.ts index 66197719..efa1dc02 100644 --- a/src/tools/search.ts +++ b/src/tools/search.ts @@ -183,16 +183,15 @@ const schema = { .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.', + 'Response format. Default `text-v1` — compact line-oriented output. Pass `format: "json"` for the structured envelope. `text` is an alias for `text-v1`. The text format is a public, snapshot-tested contract.', ), }; const DESCRIPTION = "Search indexed dependency and repository code, docs, and explicit symbols. " + "Provide either `target` for one target or `targets` for many; omit `sources` to use backend AUTO. " + - "The query field uses GitHits discovery syntax (AND/OR/parens/qualifiers; see the parameter description). " + - "Structured parameters combine with that query using AND semantics. " + - "Results are complete by default — if indexing is still running, the response carries a `searchRef` and no hits; pass it to `search_status` to follow up. " + + "Structured parameters combine with the `query` using AND semantics. " + + "Complete by default — if indexing is still running, the response carries a `searchRef` and no hits; pass it to `search_status` to follow up. " + "Set `allow_partial_results: true` to opt into hits from sources that finished while others continue indexing. " + "Each hit's `type` tells you the follow-up tool: `documentation_page` and `repository_doc` → `docs_read` with `locator.pageId`; `repository_code` and `repository_symbol` → `code_read` with `locator.filePath` (and `locator.startLine`/`endLine` when present)."; From 5e19ad4f278c007a0a8aba6edb8d836e79f36ec1 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Wed, 6 May 2026 15:00:07 +0300 Subject: [PATCH 2/4] feat: allow generic feedback without solution_id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The backend dropped the `solution_id` requirement on `/feedbacks`, so feedback can now be either solution-tied (anchored to a prior `get_example` result via `solution_id`) or generic (omit the field to send feedback about any tool — `search`, `code_*`, `docs_*`, `pkg_*` — or the overall experience). Surface changes: - `FeedbackParams.solutionId` is now optional. The wire body sends `solution_id: null` when absent, matching the existing `feedback_text` null-on-omit convention. - The MCP `feedback` tool schema marks `solution_id` optional and documents both modes; the parameter description explains when to anchor versus go generic, and `feedback_text` notes it is strongly recommended in generic mode. - The CLI `feedback` command takes `[solution_id]` as an optional positional. Help text and examples cover both modes. - The MCP server-level decision tree (bucket 4) is broadened to call out generic-mode coverage of the indexed tool surface. Tests gain generic-mode cases at every layer (service-level wire shape, MCP tool handler, CLI action). 1629 pass, smoke clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/commands/feedback.test.ts | 26 +++++++++++++++ src/commands/feedback.ts | 50 +++++++++++++++++++--------- src/commands/mcp-instructions.ts | 2 +- src/services/githits-service.test.ts | 21 ++++++++++++ src/services/githits-service.ts | 9 +++-- src/tools/feedback.test.ts | 26 +++++++++++++++ src/tools/feedback.ts | 19 +++++++---- 7 files changed, 128 insertions(+), 25 deletions(-) diff --git a/src/commands/feedback.test.ts b/src/commands/feedback.test.ts index 2f405219..3231657f 100644 --- a/src/commands/feedback.test.ts +++ b/src/commands/feedback.test.ts @@ -112,6 +112,32 @@ describe("feedbackAction", () => { consoleSpy.mockRestore(); }); + it("submits generic feedback when solution_id positional is omitted", async () => { + const submitFn = mock(() => + Promise.resolve({ + success: true, + message: "Feedback submitted successfully", + }), + ); + const deps = createDeps({ + githitsService: createMockGitHitsService({ submitFeedback: submitFn }), + }); + const consoleSpy = spyOn(console, "log").mockImplementation(() => {}); + + await feedbackAction( + undefined, + { accept: true, message: "code_grep regex is fast on npm:lodash" }, + deps, + ); + + expect(submitFn).toHaveBeenCalledWith({ + solutionId: undefined, + accepted: true, + feedbackText: "code_grep regex is fast on npm:lodash", + }); + consoleSpy.mockRestore(); + }); + it("exits with error when neither --accept nor --reject", async () => { const errorSpy = spyOn(console, "error").mockImplementation(() => {}); const exitSpy = spyOn(process, "exit").mockImplementation(() => { diff --git a/src/commands/feedback.ts b/src/commands/feedback.ts index 9a391bdd..60f3e5d3 100644 --- a/src/commands/feedback.ts +++ b/src/commands/feedback.ts @@ -18,9 +18,14 @@ export interface FeedbackDependencies { /** * Core feedback logic, separated for testability. + * + * `solutionId` is optional: when present, feedback is anchored to a + * specific `get_example` result; when omitted, the feedback is + * generic (covers code/package navigation, search, docs, or the + * overall experience). */ export async function feedbackAction( - solutionId: string, + solutionId: string | undefined, options: FeedbackOptions, deps: FeedbackDependencies, ): Promise { @@ -55,15 +60,23 @@ export async function feedbackAction( } } -const FEEDBACK_DESCRIPTION = `Submit feedback on a search result. +const FEEDBACK_DESCRIPTION = `Submit feedback on a tool result or the GitHits experience. -Rate whether a code example was helpful. Use --accept for positive -feedback or --reject for negative. Optionally add a message. +Two modes: + - Solution-tied: pass the [solution_id] from a prior 'githits example' + result (shown at the bottom of the markdown / under 'solution_id' + in --json) to anchor feedback to that specific result. + - Generic: omit [solution_id] to send feedback about any command + (search, pkg, docs, code) or the overall experience. A --message + is strongly recommended here. + +Use --accept for positive feedback or --reject for negative. Examples: githits feedback abc123 --accept githits feedback abc123 --reject -m "Example was outdated" - githits feedback abc123 --accept --message "Solved my problem" --json`; + githits feedback --accept -m "code_grep regex is fast on npm:lodash" + githits feedback --reject -m "search missing kotlin support"`; /** * Register the feedback command on the given program. @@ -72,20 +85,25 @@ Examples: export function registerFeedbackCommand(program: Command) { program .command("feedback") - .summary("Submit feedback on a search result") + .summary("Submit feedback on a tool result or the GitHits experience") .description(FEEDBACK_DESCRIPTION) - .argument("", "Solution ID from search result") + .argument( + "[solution_id]", + "Solution ID from a prior 'githits example' result (omit for generic feedback)", + ) .addOption(new Option("--accept", "Mark as helpful").conflicts("reject")) .addOption(new Option("--reject", "Mark as unhelpful").conflicts("accept")) .option("-m, --message ", "Feedback explanation") .option("--json", "Output as JSON for piping") - .action(async (solutionId: string, options: FeedbackOptions) => { - try { - const deps = await createContainer(); - await feedbackAction(solutionId, options, deps); - } catch (error) { - if (error instanceof AuthRequiredError) process.exit(1); - throw error; - } - }); + .action( + async (solutionId: string | undefined, options: FeedbackOptions) => { + try { + const deps = await createContainer(); + await feedbackAction(solutionId, options, deps); + } catch (error) { + if (error instanceof AuthRequiredError) process.exit(1); + throw error; + } + }, + ); } diff --git a/src/commands/mcp-instructions.ts b/src/commands/mcp-instructions.ts index 7cc90f05..0c02ed7d 100644 --- a/src/commands/mcp-instructions.ts +++ b/src/commands/mcp-instructions.ts @@ -18,7 +18,7 @@ const CORE_BLOCK = `GitHits provides verified, canonical code examples from glob - Need a canonical example or cross-project pattern with no specific package pinned (you're stuck, repeated attempts failed, the user wants up-to-date API usage, or the user mentioned GitHits) — call \`get_example\` for global solution synthesis. - Inspecting a specific known dependency or repository (stack trace points there, verifying how a particular library works, evaluating an upgrade) — call \`search\` plus the indexed code, docs, and package tools listed below. - Question is comparative across OSS projects (e.g. "how does X vs Y handle Z") or requires reading how a real codebase implements a feature — combine the two: \`search\` for known targets, \`get_example\` for cross-project synthesis. -- Rate a result so it improves future answers — call \`feedback\` with the \`solution_id\` from a prior \`get_example\` response. +- Rate a result or share tool/UX feedback — call \`feedback\` with a \`solution_id\` from a prior \`get_example\` response (solution-tied) or omit it for generic feedback about any tool (\`search\`, \`code_*\`, \`docs_*\`, \`pkg_*\`) or the overall experience. \`get_example\` workflow: pass \`language\` only when you know the exact name; otherwise call \`search_language\` first. Default output is markdown with a trailing \`solution_id\`. Reuse prior results before searching again. For dependency-specific grounding, prefer package-scoped \`search\` before global \`get_example\`.`; diff --git a/src/services/githits-service.test.ts b/src/services/githits-service.test.ts index 578ddfaf..4a21b096 100644 --- a/src/services/githits-service.test.ts +++ b/src/services/githits-service.test.ts @@ -241,6 +241,27 @@ describe("GitHitsServiceImpl", () => { ).rejects.toThrow("Server error (500): internal error"); }); + it("sends null solution_id when not provided (generic feedback)", async () => { + const fn = mockFetch(() => + Promise.resolve( + new Response(JSON.stringify({ success: true }), { + headers: { "Content-Type": "application/json" }, + }), + ), + ); + + await service.submitFeedback({ + accepted: true, + feedbackText: "code_grep regex is great", + }); + + const call = fn.mock.calls[0] as unknown as [string, RequestInit]; + const body = JSON.parse(call[1].body as string); + expect(body.solution_id).toBeNull(); + expect(body.accepted).toBe(true); + expect(body.feedback_text).toBe("code_grep regex is great"); + }); + it("sends null feedback_text when not provided", async () => { const fn = mockFetch(() => Promise.resolve( diff --git a/src/services/githits-service.ts b/src/services/githits-service.ts index 0f373038..096a6b9a 100644 --- a/src/services/githits-service.ts +++ b/src/services/githits-service.ts @@ -51,9 +51,14 @@ export interface SearchParams { /** * Parameters for feedback API call. + * + * `solutionId` is optional: when present, the feedback is anchored + * to a specific `get_example` result; when absent, the feedback is + * generic and applies to the overall tool surface (code/package + * navigation, search, docs, or the GitHits experience as a whole). */ export interface FeedbackParams { - solutionId: string; + solutionId?: string; accepted: boolean; feedbackText?: string; } @@ -130,7 +135,7 @@ export class GitHitsServiceImpl implements GitHitsService { method: "POST", headers: this.headers(), body: JSON.stringify({ - solution_id: params.solutionId, + solution_id: params.solutionId ?? null, accepted: params.accepted, feedback_text: params.feedbackText ?? null, }), diff --git a/src/tools/feedback.test.ts b/src/tools/feedback.test.ts index 80530632..296eb224 100644 --- a/src/tools/feedback.test.ts +++ b/src/tools/feedback.test.ts @@ -53,6 +53,32 @@ describe("feedbackTool", () => { }); }); + it("submits generic feedback without solution_id", async () => { + const submitFeedback = mock(() => + Promise.resolve({ + success: true, + message: "Feedback submitted successfully", + }), + ); + const service = createMockGitHitsService({ submitFeedback }); + const tool = createFeedbackTool(service); + + const result = await tool.handler( + { + accepted: false, + feedback_text: "search is missing kotlin support", + }, + {}, + ); + + expect(result.isError).toBeUndefined(); + expect(submitFeedback).toHaveBeenCalledWith({ + solutionId: undefined, + accepted: false, + feedbackText: "search is missing kotlin support", + }); + }); + it("returns error result on service failure", async () => { const service = createMockGitHitsService({ submitFeedback: mock(() => Promise.reject(new Error("Auth required"))), diff --git a/src/tools/feedback.ts b/src/tools/feedback.ts index dbb3d618..c3c57606 100644 --- a/src/tools/feedback.ts +++ b/src/tools/feedback.ts @@ -4,7 +4,7 @@ import { withErrorHandling } from "./shared.js"; import { type ToolDefinition, textResult } from "./types.js"; interface FeedbackArgs { - solution_id: string; + solution_id?: string; accepted: boolean; feedback_text?: string; } @@ -13,23 +13,30 @@ const schema = { solution_id: z .string() .min(1) + .optional() .describe( - "The `solution_id` returned by a prior `get_example` call (shown on the trailing line of the markdown result, or under the `solution_id` key in JSON mode).", + "Optional. Pass the `solution_id` from a prior `get_example` response (shown on the trailing line of the markdown result, or under the `solution_id` key in JSON mode) to anchor feedback to that specific result. Omit for generic feedback about any tool (code/package navigation, search, docs) or the overall experience.", ), accepted: z .boolean() - .describe("True if the example was helpful/good, False if unhelpful/bad"), + .describe( + "True for positive feedback (helpful/good), False for negative (unhelpful/bad). Always required.", + ), feedback_text: z .string() .optional() .describe( - 'Optional text explaining why (e.g., "This solved problem X" or "Example was outdated")', + 'Optional context (e.g., "This solved problem X" or "code_grep regex over npm:lodash missed Foo function"). Strongly recommended when `solution_id` is omitted, since there is no specific result to anchor to.', ), }; -const DESCRIPTION = `Submit feedback on a \`get_example\` result. +const DESCRIPTION = `Submit feedback on a tool result or the GitHits experience. + +Two modes: +1. **Solution-tied** — pass the \`solution_id\` from a prior \`get_example\` response to rate that specific result. +2. **Generic** — omit \`solution_id\` to send feedback about any tool (\`search\`, \`code_grep\`, \`code_read\`, \`code_files\`, \`docs_*\`, \`pkg_*\`) or the overall experience. -Call after \`get_example\` with the returned \`solution_id\`. \`accepted=true\` when the example solved the problem or was useful; \`accepted=false\` when it was irrelevant or wrong. Use \`feedback_text\` to add a short reason. Feeds ranking quality. Not for unified \`search\` hits — those have no \`solution_id\`.`; +\`accepted\` is always required (true = positive, false = negative). Add \`feedback_text\` for context — strongly recommended in generic mode. Feeds ranking and product quality.`; export function createFeedbackTool( service: GitHitsService, From 3c03edd9fa7f6317c9dc2d9a95bbdc83cb52d6ef Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Wed, 6 May 2026 17:52:49 +0300 Subject: [PATCH 3/4] chore: omit solution_id from feedback body when generic Switches the generic-feedback wire format from sending \`solution_id: null\` to omitting the key entirely. Both shapes work against the backend; key-absent is the cleaner contract since the field is now conceptually optional rather than nullable. Verified end-to-end against production: bun run ./src/cli.ts feedback --accept -m "..." -> "Feedback submitted successfully" bun run ./src/cli.ts feedback --reject -m "..." --json -> {"success":true,"message":"Feedback submitted successfully"} The service test now asserts the key is absent (\`"solution_id" in body\` is \`false\`) rather than \`null\`. Solution-tied wire shape is unchanged. \`feedback_text\` keeps null-on-omit since backend infrastructure was already shaped that way. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/services/githits-service.test.ts | 4 ++-- src/services/githits-service.ts | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/services/githits-service.test.ts b/src/services/githits-service.test.ts index 4a21b096..53d5739b 100644 --- a/src/services/githits-service.test.ts +++ b/src/services/githits-service.test.ts @@ -241,7 +241,7 @@ describe("GitHitsServiceImpl", () => { ).rejects.toThrow("Server error (500): internal error"); }); - it("sends null solution_id when not provided (generic feedback)", async () => { + it("omits solution_id when not provided (generic feedback)", async () => { const fn = mockFetch(() => Promise.resolve( new Response(JSON.stringify({ success: true }), { @@ -257,7 +257,7 @@ describe("GitHitsServiceImpl", () => { const call = fn.mock.calls[0] as unknown as [string, RequestInit]; const body = JSON.parse(call[1].body as string); - expect(body.solution_id).toBeNull(); + expect("solution_id" in body).toBe(false); expect(body.accepted).toBe(true); expect(body.feedback_text).toBe("code_grep regex is great"); }); diff --git a/src/services/githits-service.ts b/src/services/githits-service.ts index 096a6b9a..0b722ea0 100644 --- a/src/services/githits-service.ts +++ b/src/services/githits-service.ts @@ -131,11 +131,17 @@ export class GitHitsServiceImpl implements GitHitsService { async submitFeedback(params: FeedbackParams): Promise { return withTelemetrySpan("githits.feedback.request", async () => { + // `solution_id` is omitted entirely when undefined — the backend + // accepts the key absent for generic feedback. `feedback_text` + // still uses null-on-omit because backend infrastructure was + // already shaped that way; revisit if/when it changes. const response = await fetch(`${this.apiUrl}/feedbacks`, { method: "POST", headers: this.headers(), body: JSON.stringify({ - solution_id: params.solutionId ?? null, + ...(params.solutionId !== undefined && { + solution_id: params.solutionId, + }), accepted: params.accepted, feedback_text: params.feedbackText ?? null, }), From bc11e7f1c3271522630928e3fa9098447b0d6983 Mon Sep 17 00:00:00 2001 From: Juha Litola Date: Thu, 7 May 2026 08:21:13 +0300 Subject: [PATCH 4/4] fix: address agent-UX gaps surfaced by live MCP testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small fixes from running the new instructions against a real agent session, each chosen to improve agent UX without inflating tool descriptions: 1. Dedupe identical warnings in `search` envelopes. When N hits share a target+freshness state, the per-hit freshness composer used to emit the same string N times ("served stale npm:zod@4.4.3 while ... indexes" repeated five times in the live test). The `combineWarnings` aggregator now collapses through a Set, so the agent sees one signal per condition while parser warnings stay pinned at the head. 2. `code_read` cap-hint suggests the next `start_line` concretely. When a read is truncated, the existing hint reported what was returned vs. requested but left the agent to compute the next call. Now appends "To continue, retry with start_line=${endLine + 1}." — same hint surface, no schema change, just removes the math on the agent's side. 3. `search` no-target error names both parameters. The validation already fires client-side via `resolveTargets`, but the message said only "At least one target is required." Now reads "Provide either `target` for one search target or `targets` for multiple; neither was set." so an agent reading the envelope can fix the call without re-reading the tool description. Tests updated and added for each change. 1630 pass, smoke and build clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/shared/unified-search-request.ts | 6 ++-- src/shared/unified-search-response.test.ts | 35 ++++++++++++++++++++++ src/shared/unified-search-response.ts | 7 ++++- src/tools/read-file.test.ts | 2 ++ src/tools/read-file.ts | 10 ++++++- src/tools/search.test.ts | 7 +++-- 6 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/shared/unified-search-request.ts b/src/shared/unified-search-request.ts index 67d88bc9..5f4b5ef0 100644 --- a/src/shared/unified-search-request.ts +++ b/src/shared/unified-search-request.ts @@ -82,13 +82,15 @@ function resolveTargets( ): CodeNavigationTarget[] { if (target && targets) { throw new InvalidArgumentError( - "Provide either target or targets, not both.", + "Provide either `target` for one search target or `targets` for multiple, not both.", ); } const resolved = target ? [target] : (targets ?? []); if (resolved.length === 0) { - throw new InvalidArgumentError("At least one target is required."); + throw new InvalidArgumentError( + "Provide either `target` for one search target or `targets` for multiple; neither was set.", + ); } const deduped: CodeNavigationTarget[] = []; diff --git a/src/shared/unified-search-response.test.ts b/src/shared/unified-search-response.test.ts index 9fcafca8..e5c3b6b5 100644 --- a/src/shared/unified-search-response.test.ts +++ b/src/shared/unified-search-response.test.ts @@ -178,6 +178,41 @@ describe("buildUnifiedSearchSuccessPayload", () => { ); }); + it("dedupes identical freshness warnings across hits sharing a state", () => { + if (defaultUnifiedSearchOutcome.state !== "completed") { + throw new Error("expected completed outcome fixture"); + } + const baseHit = defaultUnifiedSearchOutcome.result.results[0]!; + // Five hits all stale on the same target — agents should see one + // warning, not five copies of the same string. + const staleHit = { + ...baseHit, + requestedTargetLabel: "npm:zod latest", + freshTargetLabel: "npm:zod@4.4.4", + servedTargetLabel: "npm:zod@4.4.3", + freshness: "STALE" as const, + }; + const outcome: UnifiedSearchOutcome = { + ...defaultUnifiedSearchOutcome, + result: { + ...defaultUnifiedSearchOutcome.result, + results: [staleHit, staleHit, staleHit, staleHit, staleHit], + }, + }; + + const payload = buildUnifiedSearchSuccessPayload( + params, + "schema", + "schema", + outcome, + ); + + const matches = (payload.warnings ?? []).filter((entry) => + entry.includes("served stale npm:zod@4.4.3"), + ); + expect(matches).toHaveLength(1); + }); + it("omits non-actionable current freshness metadata from hits", () => { if (defaultUnifiedSearchOutcome.state !== "completed") { throw new Error("expected completed outcome fixture"); diff --git a/src/shared/unified-search-response.ts b/src/shared/unified-search-response.ts index 0aa60873..3c290b43 100644 --- a/src/shared/unified-search-response.ts +++ b/src/shared/unified-search-response.ts @@ -268,7 +268,12 @@ function combineWarnings( out.push(...buildHitFreshnessWarnings(hits)); out.push(...buildProgressFreshnessWarnings(progress)); out.push(...buildSourceStatusWarnings(sourceStatus)); - return out; + // Hit-level freshness warnings collapse to the same string when N + // hits share a target+freshness state ("served stale npm:zod@4.4.3 + // while ... indexes" repeated per-hit). Dedupe at the envelope so + // the agent sees one signal per condition. Set preserves + // first-occurrence order, keeping parser warnings at the head. + return Array.from(new Set(out)); } export function buildUnifiedSearchErrorPayload( diff --git a/src/tools/read-file.test.ts b/src/tools/read-file.test.ts index 5aba069b..071cb81e 100644 --- a/src/tools/read-file.test.ts +++ b/src/tools/read-file.test.ts @@ -409,6 +409,8 @@ describe("createReadFileTool — span cap", () => { 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"); + // Concrete next-call suggestion removes the math-on-the-agent. + expect(payload.hint).toContain("retry with start_line=151"); expect(payload.hint).toContain("80-150 lines"); expect(payload.hint).toContain("retry also costs context"); }); diff --git a/src/tools/read-file.ts b/src/tools/read-file.ts index e1d2911a..3d35d1b1 100644 --- a/src/tools/read-file.ts +++ b/src/tools/read-file.ts @@ -227,9 +227,17 @@ function buildCappedHint( originalEnd: number | undefined, ): string { const requested = describeRequest(originalStart, originalEnd); + // Suppress the bare end-line if `endLine` is missing — exhaustive + // suppression already happens upstream in `shouldEmitCappedHint`, + // but we read `endLine` defensively here. + const continuation = + payload.endLine !== undefined + ? ` To continue, retry with start_line=${payload.endLine + 1}.` + : ""; return ( `Returned lines ${payload.startLine}-${payload.endLine}/${payload.totalLines} ` + - `(MCP cap: ${MCP_READ_MAX_SPAN} lines per call; you requested ${requested}). ` + + `(MCP cap: ${MCP_READ_MAX_SPAN} lines per call; you requested ${requested}).` + + `${continuation} ` + `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.` ); diff --git a/src/tools/search.test.ts b/src/tools/search.test.ts index b2d21712..519ec0bd 100644 --- a/src/tools/search.test.ts +++ b/src/tools/search.test.ts @@ -114,9 +114,10 @@ describe("searchTool", () => { const result = await tool.handler({ query: "test" }, {}); expect(result.isError).toBe(true); - expect(result.content[0]?.text).toContain( - "At least one target is required", - ); + // Error names both parameters so an agent can fix the call without + // re-reading the description. + expect(result.content[0]?.text).toContain("`target`"); + expect(result.content[0]?.text).toContain("`targets`"); }); it("preserves omitted repo refs for backend default-branch discovery", async () => {