From 68020734df935e6b99efb0096bf9996702b9282e Mon Sep 17 00:00:00 2001 From: Aymen Hammouda Date: Sat, 3 Oct 2026 08:46:14 +0200 Subject: [PATCH] Improve section ranking for canonical Python documentation (#134) --- docs/architecture/DESIGN.md | 11 ++ .../retrieval/ranker.py | 62 +++++++++- tests/test_retrieval.py | 109 ++++++++++++++++++ 3 files changed, 176 insertions(+), 6 deletions(-) diff --git a/docs/architecture/DESIGN.md b/docs/architecture/DESIGN.md index e887ff9..17511ca 100644 --- a/docs/architecture/DESIGN.md +++ b/docs/architecture/DESIGN.md @@ -317,6 +317,17 @@ pure functions with no storage or MCP imports: carrying a `snippet()`-generated excerpt; an `sqlite3.OperationalError` from a malformed match expression is caught, logged, and returns an empty list rather than raising. +- Section result slots are filled after examining a ranked candidate window. A + same-version, same-page overview and API section compete for one slot only + when one stored section contains the other **and** their highlighted FTS + excerpts share at least 70% of the shorter excerpt (minimum eight words). + The narrower canonical anchor wins for that matched passage. An overview + with independently matching introductory text, or a distinct topic on the + same page, remains a separate hit. This changes search result selection only: + direct symbol/anchor lookup and whole-page `get_docs` content are unchanged. + The entire ranked window is examined even after the result slots fill, so a + later narrow API anchor can replace an overview. The window is bounded to + `max(40, 8 * max_results)` candidates (at most 160 through the public API). - `search_symbols` (line 197) queries `symbols_fts` with column weights (`qualified_name` 10.0, `module` 1.0) the same way. - `search_examples` (line 256) queries `examples_fts` for code-sample hits. diff --git a/src/mcp_server_python_docs/retrieval/ranker.py b/src/mcp_server_python_docs/retrieval/ranker.py index f26eabc..8d3047e 100644 --- a/src/mcp_server_python_docs/retrieval/ranker.py +++ b/src/mcp_server_python_docs/retrieval/ranker.py @@ -9,7 +9,9 @@ from __future__ import annotations import logging +import re import sqlite3 +from difflib import SequenceMatcher from mcp_server_python_docs.models import SymbolHit @@ -133,6 +135,32 @@ def _normalize_scores(hits: list[SymbolHit]) -> list[SymbolHit]: return normalized +def _section_excerpt_overlaps(first: sqlite3.Row, second: sqlite3.Row) -> bool: + """Only collapse nested sections when their *matched excerpts* are redundant. + + A shared page is not enough: an overview can independently answer a broad + query while a nearby API section answers a precise one. The content and the + FTS snippet must both overlap substantially before they compete for a slot. + """ + if first["version"] != second["version"] or first["slug"] != second["slug"]: + return False + first_content = " ".join(first["content_text"].casefold().split()) + second_content = " ".join(second["content_text"].casefold().split()) + if not first_content or not second_content: + return False + if first_content not in second_content and second_content not in first_content: + return False + + first_words = re.findall(r"\w+", first["snippet_text"].casefold()) + second_words = re.findall(r"\w+", second["snippet_text"].casefold()) + shorter = min(len(first_words), len(second_words)) + if shorter < 8: + return False + shared = SequenceMatcher(None, first_words, second_words, autojunk=False) + match = shared.find_longest_match(0, len(first_words), 0, len(second_words)) + return match.size >= max(8, (7 * shorter + 9) // 10) + + def search_sections( conn: sqlite3.Connection, match_expr: str, @@ -154,10 +182,15 @@ def search_sections( Returns: List of SymbolHit with kind="section" and FTS5 snippets. """ + # Inspect a fixed, ranked window even after the result slots fill: a later + # narrow API anchor can replace an earlier enclosing overview. The server + # caps max_results at 20, so this examines at most 160 candidates and does + # not paginate through an unbounded run of overlapping sections. + candidate_limit = max(40, max_results * 8) try: - cursor = conn.execute( + candidates = conn.execute( """ - SELECT s.id, s.heading, s.uri, s.anchor, + SELECT s.id, s.heading, s.uri, s.anchor, s.content_text, d.version, doc.slug, bm25(sections_fts, 10.0, 1.0) as score, snippet(sections_fts, 1, '**', '**', '...', 32) as snippet_text @@ -167,16 +200,33 @@ def search_sections( JOIN doc_sets d ON doc.doc_set_id = d.id WHERE sections_fts MATCH ? AND (? IS NULL OR d.version = ?) - ORDER BY bm25(sections_fts, 10.0, 1.0) + ORDER BY bm25(sections_fts, 10.0, 1.0), s.id LIMIT ? """, - (match_expr, version, version, max_results), - ) - rows = cursor.fetchall() + (match_expr, version, version, candidate_limit), + ).fetchall() except sqlite3.OperationalError: logger.warning("FTS5 query failed for sections: %r", match_expr) return [] + selected: list[sqlite3.Row] = [] + for row in candidates: + duplicate = next( + (i for i, prior in enumerate(selected) + if _section_excerpt_overlaps(prior, row)), + None, + ) + if duplicate is None: + selected.append(row) + elif len(row["content_text"]) < len(selected[duplicate]["content_text"]): + # The narrower section gives a more precise canonical anchor + # for the same matched text. The overview stays available + # through its anchor and whole-page get_docs retrieval. + selected[duplicate] = row + + selected.sort(key=lambda row: (row["score"], row["id"])) + rows = selected[:max_results] + hits = [ SymbolHit( uri=row["uri"], diff --git a/tests/test_retrieval.py b/tests/test_retrieval.py index a69aaec..065a08f 100644 --- a/tests/test_retrieval.py +++ b/tests/test_retrieval.py @@ -767,3 +767,112 @@ def test_no_raw_match_in_source(): "Raw MATCH string concatenation found (RETR-02 violation):\n" + "\n".join(violations) ) + + +def test_later_api_replaces_full_window_overview_without_losing_other_topic(): + """Inspect past filled slots, but never beyond the fixed candidate window.""" + api = ( + "Cumulative totals combine each input with the previous result and " + "return every intermediate value in the original iterator order." + ) + overview = "A broad introduction to iterator utilities. " + api + snippet = ( + "**Cumulative** totals combine each input with the previous result " + "and return every intermediate value" + ) + + def row(section_id, anchor, content, score, excerpt=snippet): + return { + "id": section_id, + "heading": anchor, + "uri": f"library/itertools.html#{anchor}", + "anchor": anchor, + "content_text": content, + "version": "3.13", + "slug": "library/itertools", + "score": score, + "snippet_text": excerpt, + } + + candidates = [ + row(1, "overview", overview, -20.0), + row(2, "recipes", "Distinct grouped-iterator recipe for cumulative batches.", + -19.9, "**Cumulative** batches are collected by a distinct grouped-iterator recipe"), + ] + candidates.extend( + row(i, f"copied-{i}", overview + f" Additional note {i}.", -20 + i / 10) + for i in range(3, 21) + ) + candidates.append(row(21, "accumulate", api, -17.9)) + candidates.extend( + row(i, f"copied-{i}", overview + f" Additional note {i}.", -20 + i / 10) + for i in range(22, 41) + ) + candidates.append(row(41, "outside-window", "A separate cumulative topic.", -15.9)) + + class RankedConnection: + def __init__(self): + self.limits = [] + self.rows = [] + + def execute(self, _sql, params): + limit = params[-2] if len(params) == 5 else params[-1] + offset = params[-1] if len(params) == 5 else 0 + self.limits.append(limit) + self.rows = candidates[offset:offset + limit] + return self + + def fetchall(self): + return self.rows + + conn = RankedConnection() + hits = search_sections(conn, '"cumulative"', "3.13", 2) + assert [hit.anchor for hit in hits] == ["recipes", "accumulate"] + assert hits[0].score > hits[1].score + assert all(hit.version == "3.13" for hit in hits) + assert conn.limits == [40] + + +def test_nested_api_excerpt_does_not_consume_section_slot(fts_db): + """A copied API passage uses one slot, but another same-page topic survives.""" + from mcp_server_python_docs.services.content import ContentService + + api = ( + "Cumulative totals are produced by repeatedly combining each value " + "with the prior result while preserving the order of the input. " + "The operation returns every intermediate total to the caller." + ) + overview = "A general introduction to iterator tools.\n\n" + api + distinct = ( + "Cumulative recipes can reset a running window at group boundaries " + "and use a separate iterator to collect the resulting batches." + ) + rows = [ + (3, "overview", "Cumulative overview", overview), + (4, "accumulate", "Cumulative API", api), + (5, "recipes", "Cumulative recipes", distinct), + ] + for section_id, anchor, heading, content in rows: + fts_db.execute( + "INSERT INTO sections (id, document_id, uri, anchor, heading, level, " + "ordinal, content_text, char_count) VALUES (?, 1, ?, ?, ?, 2, ?, ?, ?)", + (section_id, f"library/asyncio-task.html#{anchor}", anchor, + heading, section_id, content, len(content)), + ) + fts_db.execute( + "UPDATE documents SET content_text = ? WHERE id = 1", (overview + "\n\n" + distinct,) + ) + fts_db.commit() + fts_db.execute("INSERT INTO sections_fts(sections_fts) VALUES('rebuild')") + + hits = search_sections(fts_db, fts5_escape("cumulative"), "3.13", 2) + assert {hit.anchor for hit in hits} == {"accumulate", "recipes"} + assert all(hit.slug == "library/asyncio-task.html" for hit in hits) + overview_hits = search_sections( + fts_db, fts5_escape("general"), "3.13", 2 + ) + assert [hit.anchor for hit in overview_hits] == ["overview"] + + content = ContentService(fts_db) + assert content.get_docs("library/asyncio-task.html", "3.13", "accumulate").content == api + assert distinct in content.get_docs("library/asyncio-task.html", "3.13").content