Add top_k and snippets options for search_docs_tool

[eltonjohnfanboy]

Description

search_docs_tool returned every match (~50 results) with each result's full text body, producing responses of ~5k–14k tokens. Inside an agent loop (where the whole conversation is re-sent each turn) a second search overflows the context window and derails the run. This makes search_docs return short, query-centered snippets by default and caps the number of results, while full page content stays available via the unchanged get_page_tool.

Linked Issue(s)

Fixes #216

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Test A
• Test B

Test Configuration:

• Firmware version:
• Hardware:
• Toolchain:
• SDK:

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[CLAassistant]

All committers have signed the CLA.

[cbjuan]

Review

What it does: search_docs_tool previously returned all ~50 upstream matches with each result's full page body (~5k–14k tokens), overflowing the context window when called repeatedly inside an agent loop. This PR returns short, query-centered snippets by default, caps results to top_k (default 5, max 10), adds a detail="full" escape hatch, and whitelists result fields. Full page content stays available via the unchanged get_page_tool.

Overall: Solid, well-tested work. mypy is clean, all 171 tests pass, the snippet-windowing math (_make_snippet / _densest_window_start / _trim_partial_words) holds up under adversarial inputs, there are no external callers of the changed functions, and all new imports/constants resolve. No blocking correctness bugs. The items below are design/compat and hardening points worth a conscious sign-off rather than crashes.

Findings

1. data_fetcher.py — total_results silently changed meaning (returned-count vs total-match-count).
Previously total_results == len(cleaned) == all matches. Now it equals min(matches, top_k), with the true total moved to the new total_matches. The key name is unchanged but its value's semantics changed. An external consumer that renders "Found N results" from total_results will now under-report (e.g. "5" when 200 matched). The README was updated to read total_matches, so nothing in-repo breaks — but consider whether keeping total_results as the grand total (and naming the capped one e.g. returned_results) would be less surprising for existing clients.

2. Default-mode response is a wire-format breaking change, but the PR is checked "non-breaking".
Default detail="snippet" drops the text field (replaced by snippet) and whitelists fields to _RESULT_META_FIELDS, dropping language, package, and other upstream keys. This is the intended, documented goal — but an external MCP client reading result["results"][i]["text"] (or language/package) now gets a missing field. The "Breaking change" box is unchecked; this warrants a changelog/release note and a conscious decision on classification.

3. _make_snippet — empty-phrase anchors at offset 0 instead of falling through to term clustering.
haystack.find(phrase) returns 0 (not -1) when phrase == "", so a whitespace-only query would short-circuit to a head-anchored snippet and skip the term-cluster/head-fallback logic. Currently latent — search_qiskit_docs strips and rejects blank queries upstream — but _make_snippet is now an independently tested helper. A one-line guard (if phrase and haystack.find(phrase) != -1) closes it cheaply.

4. Env-configured top_k bounds have no lower-bound validation.
The runtime top_k param is clamped for non-positive values, but MAX_SEARCH_TOP_K/DEFAULT_SEARCH_TOP_K read from env are not. Setting QISKIT_DOCS_MAX_SEARCH_TOP_K=-1 yields effective_top_k = min(5, -1) = -1 → results[:-1], silently dropping the last result while still reporting total_matches/truncated. Operator-misconfiguration only (not user-reachable), so low severity, but _get_env_int accepting negatives is inconsistent with the careful runtime clamp.

None of these block merge. #1 and #2 are the two worth an explicit decision (API-compat surface); #3 and #4 are cheap hardening.

🤖 Generated with Claude Code

[cbjuan]

Re-review (after fix commit 2bedb39)

Thanks for the quick turnaround — all four points from the previous review are addressed cleanly, with regression tests for each. Verified against the PR head: 163 tests pass (2 new), mypy clean, no stale key references.

#	Previous finding	Resolution
1	total_results silently changed meaning	total_results now restored to the grand total of matches; the capped count moved to a new returned_results key. Docstrings, note text, and README example all updated consistently. ✓
2	Default-mode wire-format break not documented	README now carries an explicit behavior-change note describing snippet-by-default, the field whitelist, and detail="full" / get_page_tool for full content. ✓
3	_make_snippet empty-phrase anchored at offset 0	Guarded with anchor = haystack.find(phrase) if phrase else -1; an empty phrase now falls through to term-clustering / head fallback. Covered by test_blank_query_falls_back_to_head. ✓
4	Env-configured top_k bounds unvalidated	Constants floored (max(1, …) for the top_k knobs, max(50, …) for snippet chars) and effective_top_k = max(1, min(…)) added so a misconfigured negative cap can't produce a results[:-1] slice. Covered by test_misconfigured_max_does_not_negative_slice. ✓

No new issues introduced by the fix. LGTM.

[jesus-talavera-ibm]

LGTM! Thanks