Skip to content

fix(mcp): tighten tool and input descriptions - #3396

Open
brandon-pereira wants to merge 11 commits into
mainfrom
agent/mcp-tool-descriptions
Open

brandon-pereira wants to merge 11 commits into
mainfrom
agent/mcp-tool-descriptions

Conversation

@brandon-pereira

@brandon-pereira brandon-pereira commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Summary

Claude Code truncates MCP tool descriptions longer than 2048 UTF-8 bytes. A tool-doc review also found inputs with no description, descriptions that repeat schema constraints (defaults, ranges, enum lists), ID fields that don't say where the ID comes from, and tools that don't describe their mutations or return values.

Nearly all changes are text only:

  • Shortened four tool descriptions that were over the limit and moved per-field detail into the field descriptions: clickstack_event_deltas (3318 → 1796 bytes), trace_top_time_consuming_operations (2585 → 1436), trace_waterfall (2199 → 1244), table (2082 → 1578).
  • Added descriptions to every input that lacked one: save_source fields, dashboard tile config/containers/colorRules/backgroundChart, alert chart-level whereLanguage, and the event_deltas baseline fields.
  • Removed descriptions that only repeated schema constraints. ID fields now say which tool returns the ID.
  • save_alert, save_webhook, save_saved_search and patch_dashboard now describe how updates work and what side effects they have. For example, patch_dashboard deletes the alerts on a tile when the tile's new config can't hold alerts. get_dashboard_tile and query_tile now describe what they return.
  • Fixed wrong descriptions: the percent number format multiplies by 100, and heatmapScaleType sets the Y-axis scale.

Non-text changes. Validation behavior doesn't change:

  • Field descriptions for the number-tile color rules, BackgroundChartSchema, and DashboardContainerSchema / DashboardContainerTabSchema are added in common-utils/src/types.ts, so the MCP tools use those schemas unchanged. Other consumers get the descriptions too.
  • query/eventDeltas builds the target and baseline groups from a small factory so each group gets its own descriptions. Types and defaults are unchanged.

Not done:

  • outputSchema, because tools return JSON as text content, not structuredContent.
  • The remaining "missing description" findings on $ref dedupe pointers. These come from the scanner, not from missing descriptions.

packages/api/src/mcp/__tests__/toolDescriptions.test.ts fails if any tool description or the server instructions are empty or go over 2048 bytes, or if any input property lacks a description (bare $ref pointers excepted).

How to reproduce

Starting point: local yarn dev.

On main:

  1. Run yarn dev.
  2. Connect npx @modelcontextprotocol/inspector to the local MCP endpoint (see MCP.md).
  3. Look at the initialize and tools/list responses. clickstack_event_deltas and the trace tools have descriptions over 2048 bytes, and several inputs have no description.

On this branch:

  1. Repeat the steps above. Every tool description is under 2048 bytes and every input has a description.

References

  • Related PRs: independent of agent/mcp-tool-annotations

@vercel

vercel Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
hyperdx-oss Ready Ready Preview Oct 16, 2026 10:53pm UTC
hyperdx-storybook Ready Ready Preview Oct 16, 2026 10:53pm UTC

Request Review

@changeset-bot

changeset-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 17d1449

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@hyperdx/api Patch
@hyperdx/common-utils Patch
@hyperdx/app Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Duplicate code analysis

No new duplicate code introduced by this PR.


jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 124 in total (1.68%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issues remain.

Summary

The PR shortens MCP descriptions, fills in missing input descriptions, and explains updates and side effects.

  • Query and trace tools explain their inputs and results more clearly.
  • Dashboard tools explain tile settings and update effects more clearly.
  • Alert and saved-search tools explain how updates affect stored settings.
  • Source tools explain how source settings map to telemetry data.
  • The API Zod helper stops re-exporting two shared tag limits.

Reviews (11) · Last reviewed commit: "fix(mcp): clarify saved-search whereLang..." · Reviewed by Greptile

Comment thread packages/api/src/mcp/tools/query/schemas.ts Outdated
Comment thread packages/api/src/mcp/tools/query/eventDeltas.ts
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 430 passed • 1 skipped • 1335s

Status Count
✅ Passed 430
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

@brandon-pereira
brandon-pereira marked this pull request as ready for review October 16, 2026 19:44
@github-actions github-actions Bot added the review/tier-4 Critical — deep review + domain expert sign-off label Oct 16, 2026
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

🔴 Tier 4 — Critical

Touches authentication, tenancy data models, the public API or shipped database config — or substantially changes the query rendering engine, background tasks, the OTel pipeline, image build, or release CI.

Why this tier:

  • Large diff: 2330 production lines changed (threshold: 1000)
  • Cross-layer change: touches backend (packages/api) + shared utils (packages/common-utils)

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats
  • Production files changed: 31
  • Production lines changed: 2330 (+ 89 in test files, excluded from tier calculation)
  • Branch: agent/mcp-tool-descriptions
  • Author: brandon-pereira

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

Comment thread packages/api/src/mcp/tools/alerts/saveAlert.ts Outdated
Comment thread packages/api/src/mcp/tools/alerts/schemas.ts Outdated
Comment thread packages/api/src/mcp/tools/query/table.ts Outdated
Comment thread packages/api/src/mcp/tools/dashboards/schemas.ts Outdated
Comment thread packages/api/src/mcp/tools/dashboards/schemas.ts Outdated
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

PR Review

✅ No issues found.

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Deep Review

Scope: merge-base (1773c119)..HEAD — 34 files, ~1509/-916 lines, all under packages/api/src/mcp/** and packages/common-utils/src/types.ts (MCP tool/input description text, one schema-factory refactor, two test files). The AnnotationTooltip.tsx / HDXLineChart.module.scss / changeset-deletion changes shown when diffing the raw base: SHA are out of scope — that SHA is not an ancestor of HEAD, and those changes are absent from the real merge-base diff.
Intent: Keep MCP tool descriptions under Claude Code's 2048-byte truncation limit and give every input an accurate description, without changing validation behavior.

✅ No critical issues found.

No P0/P1/P2 issues were confirmed. The structural (non-text) changes are contract-safe, and the highest-risk description claims were verified against the backing code:

  • thresholdType semantics (alerts/schemas.ts) — the new description (above = >=, above_exclusive = >, below = <, below_or_equal = <=, between = threshold <= value <= thresholdMax) matches doesExceedThreshold in packages/api/src/tasks/checkAlerts/index.ts exactly.
  • Schedule-offset wording (alerts/saveAlert.ts) — "omitting both scheduleStartAt and scheduleOffsetMinutes keeps the stored schedule; clearing scheduleStartAt without an offset resets the offset to 0" matches makeAlert in packages/api/src/controllers/alerts.ts:246-258.
  • percent number format (common-utils/src/types.ts) — new "multiplies the value by 100 and appends %" corrects a previously self-contradictory description and matches the 0.5 → 50% example.
  • Structural changes — the common-utils/src/types.ts additions are purely additive .describe() calls (no change to z.infer, optionality, defaults, or enum membership); the eventDeltas.ts makeGroupSchema factory produces an identical Zod shape/defaults for both target and baseline groups; and the zod.ts MAX_TAG_LENGTH/MAX_TAGS re-export removal breaks no current consumer.
  • Prior review comments — all three substantive prior inline comments (level/quantile wording, score 0.1 tie-breaking boost, scheduleOffsetMinutes reset edge case) are confirmed addressed in the current code.
🔵 P3 nitpicks (4)
  • packages/api/src/mcp/__tests__/toolSchemas.test.ts — the input-description walker skips any property that is a bare $ref (Object.keys(prop).length === 1 && '$ref' in prop), trusting the referenced definition is described where defined; a reused/dedup'd schema whose definition lacks a top-level .describe() would pass silently while still exposing an undescribed property to the model.
    • Fix: When a property is a bare $ref, resolve the pointer and assert the target definition carries a non-empty description rather than skipping it.
    • kieran-typescript
  • packages/api/src/mcp/__tests__/toolSchemas.test.ts — the walker only requires descriptions on nodes reachable via a properties object, so scalar array items schemas are never required to carry a description, leaving element-level documentation unenforced.
    • Fix: Extend the walker to require descriptions on items schemas (or document that element-level descriptions are intentionally exempt).
    • kieran-typescript
  • packages/api/src/mcp/tools/query/eventDeltas.ts:41 — makeGroupSchema inlines a terse whereLanguage description that diverges from the shared WHERE_LANGUAGE_DESCRIPTION/whereLanguageSchema in the same query/ directory (which carries additional Lucene-vs-SQL guidance); the PR rewrote the inline text, deepening a pre-existing divergence.
    • Fix: Reuse whereLanguageSchema (or WHERE_LANGUAGE_DESCRIPTION) in makeGroupSchema, or add a comment noting the terser text is a deliberate byte-budget tradeoff.
    • maintainability
  • packages/api/src/mcp/__tests__/toolDescriptions.test.ts:10 — the 2048-byte limit is single-sourced here but lives only inside the test, so an over-long description regresses silently until CI runs with no authoring-time guard near tool registration.
    • Fix: Export the limit constant from a shared MCP util alongside the registration code so a future runtime/registration-time assertion can reuse it.
    • maintainability

Reviewers (5 returned): api-contract, previous-comments, kieran-typescript, maintainability, learnings-researcher.

Testing gaps:

  • The input-description walker can let a missing description escape via bare $ref pointers or scalar array items (see P3 items); both are enforcement holes, not current bugs.
  • eventDeltas.ts's two near-identical group schemas rely implicitly on not being deduplicated into a shared $ref for the per-property description test to inline-check both; memoizing the factory or making the schemas identical would silently weaken that check.

Coverage note: Synthesis was forced before 5 of the 10 dispatched reviewers returned — correctness, adversarial, testing, project-standards, and agent-native did not report in time. The highest-risk correctness claims (threshold comparison, schedule-offset, percent formatting, schema-shape preservation) were independently verified during synthesis and are sound, but the full correctness/adversarial passes are not reflected above. Given a purely description-and-docs change with contract-safe structural edits and all prior review comments resolved, residual risk is low; re-run the review if a complete correctness/adversarial pass is required before merge.

No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond

Comment thread packages/api/src/mcp/tools/alerts/saveAlert.ts Outdated
Comment thread packages/api/src/mcp/tools/alerts/schemas.ts
Comment thread packages/api/src/mcp/__tests__/toolDescriptions.test.ts
Comment thread packages/api/src/mcp/tools/dashboards/schemas.ts Outdated
Comment thread packages/api/src/mcp/tools/dashboards/schemas.ts Outdated
Comment thread packages/api/src/mcp/tools/dashboards/schemas.ts Outdated
Comment thread packages/api/src/mcp/__tests__/toolDescriptions.test.ts
Comment thread packages/api/src/mcp/tools/dashboards/schemas.ts Outdated
Comment thread packages/api/src/mcp/tools/trace/breakdownSchema.ts Outdated
Comment thread packages/api/src/mcp/tools/trace/waterfall.ts Outdated
Comment thread packages/api/src/mcp/tools/dashboards/getDashboardTile.ts Outdated
Comment thread packages/api/src/mcp/tools/alerts/schemas.ts
Comment thread packages/api/src/mcp/tools/alerts/schemas.ts
Comment thread packages/api/src/mcp/tools/dashboards/patchDashboard.ts Outdated
Comment thread packages/api/src/mcp/tools/trace/breakdownSchema.ts Outdated
Comment thread packages/api/src/mcp/__tests__/toolDescriptions.test.ts Outdated
Comment thread packages/api/src/mcp/tools/savedSearches/saveSavedSearch.ts
Comment thread packages/api/src/mcp/tools/query/eventDeltas.ts
Comment thread packages/api/src/mcp/__tests__/toolDescriptions.test.ts Outdated

This branch was successfully deployed

2 active deployments
Preview – hyperdx-oss — 17d1449c Deployed Oct 16, 2026 by vercel[bot]
Preview – hyperdx-storybook — 17d1449c Deployed Oct 16, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-4 Critical — deep review + domain expert sign-off

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant