From c96c604c03d631fd5f8adac46cb2ef80194d37a9 Mon Sep 17 00:00:00 2001 From: Raz Date: Sat, 28 Feb 2026 09:32:22 +0200 Subject: [PATCH] fix(mcp): make open(path) round-trip and return not_found details --- src/unity_docs_mcp/mcp_server.py | 6 +- src/unity_docs_mcp/tools/ops.py | 73 +++++++++++++++++++++--- tests/test_mcp_meta.py | 3 +- tests/test_ops_open_path_lookup.py | 89 ++++++++++++++++++++++++++++++ 4 files changed, 160 insertions(+), 11 deletions(-) create mode 100644 tests/test_ops_open_path_lookup.py diff --git a/src/unity_docs_mcp/mcp_server.py b/src/unity_docs_mcp/mcp_server.py index 9f2bffa..4f2b69d 100644 --- a/src/unity_docs_mcp/mcp_server.py +++ b/src/unity_docs_mcp/mcp_server.py @@ -174,7 +174,11 @@ def open( meta = _response_meta(docstore) record = docstore.open_doc(doc_id=doc_id, path=path) if not record: - return {"meta": meta} + return { + "error": "not_found", + "attempted": {"doc_id": doc_id, "path": path}, + "meta": meta, + } text = record.text_md if not full: cap = max_chars if max_chars is not None else docstore.config.mcp.open_max_chars diff --git a/src/unity_docs_mcp/tools/ops.py b/src/unity_docs_mcp/tools/ops.py index 29e6881..8265777 100644 --- a/src/unity_docs_mcp/tools/ops.py +++ b/src/unity_docs_mcp/tools/ops.py @@ -28,6 +28,8 @@ def __init__(self, config: Config): self.config = config self.paths = make_paths(config) self.corpus = self._load_corpus(self.paths.baked_dir / "corpus.jsonl") + self._origin_path_index = self._build_origin_path_index(self.corpus) + self._canonical_url_index = self._build_canonical_url_index(self.corpus) self._doc_source_type_counts = self._count_source_types(self.corpus.values()) self.link_index = self._load_links(self.paths.baked_dir / "link_graph.jsonl") self.searcher = HybridSearcher(config, self.paths.index_dir) @@ -60,6 +62,41 @@ def _load_links(self, path: Path) -> Dict[str, List[str]]: links.setdefault(row["from_doc_id"], []).append(row["to_doc_id"]) return links + def _build_origin_path_index(self, records: Dict[str, DocRecord]) -> Dict[str, str]: + index: Dict[str, str] = {} + for doc in records.values(): + if not doc.origin_path: + continue + index[doc.origin_path] = doc.doc_id + index[self._normalize_path_lookup_key(doc.origin_path)] = doc.doc_id + return index + + def _build_canonical_url_index(self, records: Dict[str, DocRecord]) -> Dict[str, str]: + index: Dict[str, str] = {} + for doc in records.values(): + if doc.canonical_url: + index[doc.canonical_url.strip()] = doc.doc_id + return index + + @staticmethod + def _normalize_path_lookup_key(path: str) -> str: + return path.strip().replace("\\", "/").lower() + + @staticmethod + def _maybe_doc_id_from_path(path: str) -> Optional[str]: + cleaned = path.strip().replace("\\", "/") + if not cleaned: + return None + if cleaned.lower().startswith(("http://", "https://")): + return None + if "/" in cleaned and cleaned.lower().startswith("documentation/en/"): + cleaned = cleaned[len("Documentation/en/") :] + if cleaned.lower().endswith(".html"): + cleaned = cleaned[:-5] + if "/" not in cleaned: + return None + return cleaned.replace(" ", "-").lower() + @staticmethod def _count_source_types(rows) -> Dict[str, int]: counts: Dict[str, int] = {} @@ -74,15 +111,33 @@ def _count_source_types(rows) -> Dict[str, int]: return counts def open_doc(self, doc_id: Optional[str] = None, path: Optional[str] = None) -> Optional[DocRecord]: - target_id = doc_id - if not target_id and path: - # derive doc_id from relative path - target_id = path.replace("\\", "/") - target_id = target_id.replace(".html", "") - target_id = target_id.lower() - if not target_id: - return None - return self.corpus.get(target_id) + if doc_id: + # Prefer exact doc_id match first to preserve current behavior. + record = self.corpus.get(doc_id) or self.corpus.get(doc_id.strip().lower()) + if record: + return record + + if path: + path_key_exact = path.strip() + if path_key_exact in self._canonical_url_index: + record = self.corpus.get(self._canonical_url_index[path_key_exact]) + if record: + return record + + normalized_path = self._normalize_path_lookup_key(path) + target_id = self._origin_path_index.get(path_key_exact) or self._origin_path_index.get(normalized_path) + if target_id: + record = self.corpus.get(target_id) + if record: + return record + + maybe_doc_id = self._maybe_doc_id_from_path(path) + if maybe_doc_id: + record = self.corpus.get(maybe_doc_id) + if record: + return record + + return None def list_files(self, pattern: str, limit: int = 20) -> List[DocRecord]: matches: List[DocRecord] = [] diff --git a/tests/test_mcp_meta.py b/tests/test_mcp_meta.py index ef378cc..f460501 100644 --- a/tests/test_mcp_meta.py +++ b/tests/test_mcp_meta.py @@ -98,7 +98,8 @@ def test_open_includes_meta_even_when_missing(monkeypatch): assert found["meta"]["unity_version"] == "6000.3" assert "doc_id" in found assert missing["meta"]["unity_version"] == "6000.3" - assert "doc_id" not in missing + assert missing["error"] == "not_found" + assert missing["attempted"]["doc_id"] == "missing" def test_search_invalid_source_types_returns_actionable_error(monkeypatch): diff --git a/tests/test_ops_open_path_lookup.py b/tests/test_ops_open_path_lookup.py new file mode 100644 index 0000000..b604a9b --- /dev/null +++ b/tests/test_ops_open_path_lookup.py @@ -0,0 +1,89 @@ +import json +from pathlib import Path + +from unity_docs_mcp.config import Config, PathsConfig +from unity_docs_mcp.tools import ops + + +class _FakeSearcher: + def __init__(self, config, base_path): + self.config = config + self.base_path = base_path + self.chunk_meta = {} + + def search(self, query: str, k: int = 6, source_types=None): + return [] + + +def _cfg(tmp_path: Path) -> Config: + cfg = Config() + cfg.paths = PathsConfig( + root=str(tmp_path), + raw_zip=str(tmp_path / "raw" / "UnityDocumentation.zip"), + raw_unzipped=str(tmp_path / "raw" / "UnityDocumentation"), + baked_dir=str(tmp_path / "baked"), + index_dir=str(tmp_path / "index"), + ) + return cfg + + +def _write_jsonl(path: Path, rows: list[dict]) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + with path.open("w", encoding="utf-8") as f: + for row in rows: + f.write(json.dumps(row) + "\n") + + +def test_open_doc_accepts_origin_path_round_trip(monkeypatch, tmp_path: Path): + monkeypatch.setattr(ops, "HybridSearcher", _FakeSearcher) + cfg = _cfg(tmp_path) + _write_jsonl( + tmp_path / "baked" / "corpus.jsonl", + [ + { + "doc_id": "manual/rigidbodiesoverview", + "source_type": "manual", + "title": "Introduction to rigid body physics", + "text_md": "Body text", + "origin_path": "Documentation/en/Manual/RigidbodiesOverview.html", + "canonical_url": "https://docs.unity3d.com/6000.3/Documentation/Manual/RigidbodiesOverview.html", + } + ], + ) + _write_jsonl(tmp_path / "baked" / "link_graph.jsonl", []) + + store = ops.DocStore(cfg) + + record = store.open_doc(path="Documentation/en/Manual/RigidbodiesOverview.html") + assert record is not None + assert record.doc_id == "manual/rigidbodiesoverview" + + # Alternate path spellings should also map to the same corpus doc. + record2 = store.open_doc(path="documentation\\en\\manual\\rigidbodiesoverview.html") + assert record2 is not None + assert record2.doc_id == "manual/rigidbodiesoverview" + + +def test_open_doc_accepts_canonical_url_lookup(monkeypatch, tmp_path: Path): + monkeypatch.setattr(ops, "HybridSearcher", _FakeSearcher) + cfg = _cfg(tmp_path) + _write_jsonl( + tmp_path / "baked" / "corpus.jsonl", + [ + { + "doc_id": "manual/class-rigidbody", + "source_type": "manual", + "title": "Rigidbody component reference", + "text_md": "Body text", + "origin_path": "Documentation/en/Manual/class-Rigidbody.html", + "canonical_url": "https://docs.unity3d.com/6000.3/Documentation/Manual/class-Rigidbody.html", + } + ], + ) + _write_jsonl(tmp_path / "baked" / "link_graph.jsonl", []) + + store = ops.DocStore(cfg) + record = store.open_doc(path="https://docs.unity3d.com/6000.3/Documentation/Manual/class-Rigidbody.html") + + assert record is not None + assert record.doc_id == "manual/class-rigidbody"