QVeris tools: rename search/execute/get_by_ids to discover/invoke/ins…#104
Conversation
…pect Rename QVeris tools for LLM clarity (discover/invoke/inspect), add progressive error recovery, rate-limit awareness, truncation detection, legacy tool name aliases, and rolodex-based discovery_id reuse for the inspect→invoke closed-loop flow. - qveris_search → qveris_discover, qveris_execute → qveris_invoke, qveris_get_by_ids → qveris_inspect - Legacy tool names aliased in TOOL_NAME_ALIASES for backward compat - RolodexEntry stores discoveryId from successful invokes; inspect and invoke reuse it via resolveKnownDiscoveryId (rolodex-only source) - qveris_invoke returns structured error when discovery_id is missing - Body-level failures (success: false) now include recovery_step and attempt_number alongside exception-path failures - 429 responses classified as rate_limited with retry_after_seconds - Truncation flagged with truncated: true and hint - System prompt: structured 3-step error recovery, domain examples, inspect→invoke guidance with discovery_id recovery semantics - Updated docs, release notes, AGENTS.md template, cursor rules Made-with: Cursor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines the QVeris tool integration by enhancing clarity, robustness, and efficiency. The primary goal is to improve how LLMs interact with QVeris tools through more intuitive naming conventions, comprehensive error handling, and smarter session management. These changes aim to reduce misuse, provide clearer guidance, and enable more resilient agent behavior in dynamic environments. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This is a substantial and well-executed pull request that significantly improves the clarity and robustness of the QVeris tool integration. Renaming the tools to discover, invoke, and inspect makes their purpose much clearer for the LLM. The addition of features like progressive error recovery, rate-limit awareness, and backward compatibility through aliases demonstrates great attention to detail and production readiness. The updates to the system prompts and documentation are also excellent. I've included a couple of minor suggestions for code improvements, but overall, this is a high-quality contribution.
| success: false, | ||
| error_type: "json_parse_error", | ||
| detail: | ||
| "Missing discovery_id for qveris_invoke. Run qveris_discover first, or call qveris_inspect for a previously used tool so the session rolodex can provide the discovery_id.", | ||
| retry_hint: | ||
| "Pass discovery_id from qveris_discover/qveris_inspect. If the tool was not previously used in this session, rediscover it to obtain one.", | ||
| } satisfies QverisErrorResult); |
There was a problem hiding this comment.
The error_type is set to json_parse_error, but the actual issue is a missing discovery_id, which is more of a parameter validation error. Using json_parse_error could be misleading for the LLM or for debugging. Consider using a more appropriate error type, perhaps http_error with a synthetic status of 400 to indicate a bad request, which would be more semantically correct.
success: false,
error_type: "http_error",
status: 400,
detail:
"Missing discovery_id for qveris_invoke. Run qveris_discover first, or call qveris_inspect for a previously used tool so the session rolodex can provide the discovery_id.",
retry_hint:
"Pass discovery_id from qveris_discover/qveris_inspect. If the tool was not previously used in this session, rediscover it to obtain one.",| const failCount = (invokeFailureCount.get(toolId) ?? 0) + 1; | ||
| invokeFailureCount.set(toolId, failCount); | ||
| const recoveryStep = | ||
| failCount === 1 ? "fix_params" : failCount === 2 ? "simplify" : "switch_tool"; |
There was a problem hiding this comment.
- Broaden QVeris scope to cover historical sequences, structured reports, web extraction, PDF workflows, and media processing/generation - Add paired bilingual (Chinese/English) query rewrite examples to system prompt, tool schema, and documentation - Dynamically gate prompt sections based on actual tool availability (hasInvoke, hasWebSearch, hasWebFetch) to avoid instructing on unavailable tools - Add search_id legacy alias to QverisInvokeSchema for backward compat - Deduplicate qverisExecutionLine (was inserted twice), merge step 2/3 shared guidance, compact anti-patterns — ~24% fewer routing prompt lines - Trim QverisDiscoverSchema.query description (~55% shorter) — detailed teaching stays in system prompt, schema is concise reinforcement - Reorder toolOrder to place QVeris tools before web_search/web_fetch - Conditionally narrow web_search summary when QVeris is available - Update tool-catalog brief descriptions for clarity - Update tests with bilingual and availability-gating assertions - Update README.md, docs/qverisbot-from-source.md, AGENTS.md template Made-with: Cursor
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is a substantial and well-executed pull request that significantly improves the clarity and capability of the QVeris tool integration. The renaming of tools to discover, invoke, and inspect makes their purpose much clearer for the LLM. The addition of features like progressive error recovery, rate-limit awareness, and the reuse of discovery_id via the rolodex are excellent enhancements that will make the agent more robust and efficient. The backward compatibility for legacy tool names is also a thoughtful touch.
The documentation and tests have been updated thoroughly to reflect these changes. I've left a couple of minor suggestions in src/agents/tools/qveris-tools.ts related to code clarity and maintainability, but overall this is a very strong set of improvements.
| if (!discoveryId) { | ||
| return jsonResult({ | ||
| success: false, | ||
| error_type: "json_parse_error", |
There was a problem hiding this comment.
The error_type is set to json_parse_error for a missing discovery_id. While this is a client-side error that the LLM can fix, a more specific error type like parameter_error or validation_error would be more accurate. If adding a new error type to QverisErrorResult is feasible, it would improve the clarity of the error classification. However, the detailed detail and retry_hint are excellent and likely sufficient for the LLM to recover.
| const failCount = (invokeFailureCount.get(toolId) ?? 0) + 1; | ||
| invokeFailureCount.set(toolId, failCount); | ||
| const recoveryStep = | ||
| failCount === 1 ? "fix_params" : failCount === 2 ? "simplify" : "switch_tool"; | ||
| const classified = classifyQverisError(err); | ||
| return jsonResult({ | ||
| ...classified, | ||
| recovery_step: recoveryStep, | ||
| attempt_number: failCount, | ||
| }); | ||
| } |
There was a problem hiding this comment.
This block for handling invocation failures contains logic for incrementing the failure count and determining the next recovery step. This logic is duplicated in the else block on lines 744-757 that handles backend-reported failures. To improve maintainability and reduce redundancy, you could extract this logic into a helper function within the createQverisTools scope.
For example, you could create a helper like this:
function getNextRecoveryInfo(toolId: string) {
const failCount = (invokeFailureCount.get(toolId) ?? 0) + 1;
invokeFailureCount.set(toolId, failCount);
const recoveryStep =
failCount === 1 ? "fix_params" : failCount === 2 ? "simplify" : "switch_tool";
return { recovery_step: recoveryStep, attempt_number: failCount };
}And then use it in both places to simplify the code.
…main support
- Rename qveris_invoke to qveris_call across tools, schemas, prompts, and tests
- Implement V3 full-content auto-materialization: binary-safe download via
readResponseBuffer, tryDecodeUtf8 for safe text derivation, file saving
with manifest metadata (schema, preview, analysis) returned to model
- Security: HTTPS-only, strict domain whitelist, no redirects, opt-in default
(autoMaterializeFullContent=false), size limits with accurate truncation
detection (exact-boundary peek)
- Dual-domain: region config ("global"→qveris.ai, "cn"→qveris.cn) with
dynamic base URL, whitelist derivation, and region-aware onboarding
- System prompt: conditional guidance based on autoMaterialize flag;
text→web_fetch vs binary→exec+curl fallback when materialization is off
- Fix qveris_inspect endpoint path: /tools/get-by-ids → /tools/by-ids
Made-with: Cursor
The model no longer needs to pass or track discovery_id / search_id. The integration layer auto-resolves the backend search_id from the discover tracker (keyed by tool_id after qveris_discover) and falls back to the rolodex (keyed by successful prior calls). - DiscoverResultMeta now stores searchId; trackResults() accepts it - resolveKnownSearchId() checks rolodex then discoverTracker - QverisCallSchema: remove discovery_id and search_id parameters - qveris_call handler uses resolveKnownSearchId exclusively - Add "tool_not_discovered" error type for cleaner diagnostics - Remove discovery_id from all model-facing output: discover payload, formatToolForModel, rolodex getSummary, inspect result - qveris_inspect: replace invoke_hint with call_hint (no session context) - System prompt: remove all discovery_id references from routing rules and tool summaries - Tests: add makeDiscoverResponse/registerToolViaDiscover helpers; all qveris_call tests now go through discover first Made-with: Cursor
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
…features Made-with: Cursor
There was a problem hiding this comment.
Code Review
This pull request is a significant refactoring that renames the QVeris tools to discover, call, and inspect for better LLM clarity. It also introduces several valuable features like progressive error recovery, rate-limit awareness, truncation detection, and automatic materialization of large content. The changes are well-tested and the new features are robust.
My review found two main areas for improvement:
- There is a naming inconsistency for the
qveris_calltool, which is referred to asqveris_invokein some documentation files. This should be standardized. - The system prompt instruction for the
qveris_inspectworkflow is confusing and could lead to inefficient behavior by the agent.
Overall, this is a great set of improvements. Addressing these points will enhance clarity and agent performance.
| | Tool | When to use | | ||
| | :---------------- | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `qveris_discover` | Find specialized API/service tools for exact current values, historical sequence data, structured reports, web extraction, PDF workflows, or external professional services or media processing/generation (OCR, speech, image, video, translation, map, navigation, geocoding, etc.). **Preferred over `web_search`** when a specialized provider can return the answer or perform the work. Query in English describing the API capability needed. | | ||
| | `qveris_invoke` | Call a QVeris API/service tool to get structured data, reports, extracted web content, PDFs, or processed/generated media. Requires `tool_id` and `discovery_id` from `qveris_discover` or `qveris_inspect`. | |
There was a problem hiding this comment.
There's an inconsistency in the new tool name for executing a QVeris tool. This file and the release notes use qveris_invoke, but the codebase (e.g., src/agents/tool-catalog.ts) and other documentation files use qveris_call. To avoid confusion for developers and LLMs, this should be standardized. Since the implementation consistently uses qveris_call, I recommend updating the documentation to match.
| | `qveris_invoke` | Call a QVeris API/service tool to get structured data, reports, extracted web content, PDFs, or processed/generated media. Requires `tool_id` and `discovery_id` from `qveris_discover` or `qveris_inspect`. | | |
| | `qveris_call` | Call a QVeris API/service tool to get structured data, reports, extracted web content, PDFs, or processed/generated media. Requires `tool_id` and `discovery_id` from `qveris_discover` or `qveris_inspect`. | |
| ? " -> Prefer qveris_discover + qveris_call. Specialized APIs/services return precise structured data or service outputs from dedicated providers." | ||
| : " -> Use qveris_discover to identify the best specialized API/service available in this session. If qveris_call is unavailable here, report the limitation honestly instead of promising a tool call you cannot make."; | ||
| const inspectLine = hasInspect | ||
| ? " -> Use qveris_inspect with the known tool_id to verify availability and get current parameter schemas. If the tool is available, run qveris_discover to register it for this session, then call it." |
There was a problem hiding this comment.
The guidance for using qveris_inspect is confusing and promotes an inefficient workflow. It instructs the agent to use inspect and then immediately use discover before call, without explaining why both steps are necessary. This can be confusing for the LLM, which might wonder why it shouldn't just use discover directly.
The documentation in README.md also describes a different, more ideal flow that doesn't seem to match the implementation, adding to the confusion.
To improve clarity, consider revising this instruction to better explain the purpose of each step in the context of the agent's goals.
| ? " -> Use qveris_inspect with the known tool_id to verify availability and get current parameter schemas. If the tool is available, run qveris_discover to register it for this session, then call it." | |
| ? " -> If you have a tool_id but no discovery_id for the current session, use qveris_inspect to check the tool's details. If it's still suitable, you must then use qveris_discover to get a valid discovery_id before you can call it." |
…pect
Rename QVeris tools for LLM clarity (discover/invoke/inspect), add progressive error recovery, rate-limit awareness, truncation detection, legacy tool name aliases, and rolodex-based discovery_id reuse for the inspect→invoke closed-loop flow.