From 630c2f10a57ae9410317d2587ecdd62e2348af32 Mon Sep 17 00:00:00 2001 From: cdeust Date: Wed, 29 Jul 2026 22:40:57 +0200 Subject: [PATCH 1/6] fix(types): narrow ast_parser's language str to a Literal, closing the resolver-dependent typecheck gate (#249) mcp_server/core/ast_parser.py:88 passed a plain `str` to tree_sitter_language_pack.get_parser(), whose declared parameter type differs across releases (a `SupportedLanguage` Literal in 1.6.2 vs plain `str` in 1.13.5) -- so pyright's zero-diagnostic verdict was a property of which version pip/uv resolved, not of this source. AST_SUPPORTED is now a Literal union narrowed via a TypeGuard instead of a bare set[str], verified clean under both the current uv.lock resolution and a standalone 1.6.2 install (the only residual error there is 1.6.2's own SupportedLanguage omitting "csharp" -- a pre-existing grammar-availability gap in that release, not a narrowing defect). Boy-scout pass on the touched file (mutation-tested, tools/mutation_check.sh equivalent, scoped to ast_parser.py + ast_extractor_registry.py): removed the flat `calls` list every per-language extractor computed and parse_file_ast immediately discarded (superseded by calls_per_function, never itself consumed), and closed the resulting zero test coverage for Swift/Rust parsing plus gaps in is_available, _node_text, _extract_module_doc, content_hash length, and calls_per_function's populated content. Filed #269 for the 50 mutmut "survived" reports left after that pass, which are proven false positives (verified directly via MUTANT_UNDER_TEST against the full test selection): mutmut's coverage attribution undercounts callers of a dispatch table built once, eagerly, at module-import time. Test count 6526 -> 6549 (+23), resynced across README/CONTRIBUTING/ CLAUDE.md/ASSURANCE-CASE.md/.bestpractices.json/badge-tests.svg via scripts/check_doc_claims.py + scripts/generate_repo_badges.py. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u --- CHANGELOG.md | 1 + mcp_server/core/ast_extractor_registry.py | 33 ++- mcp_server/core/ast_parser.py | 49 ++--- tests_py/core/test_ast_parser.py | 236 +++++++++++++++++++++- 4 files changed, 268 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eebabe50..8e83ff7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ adheres to [Semantic Versioning](https://semver.org/). - **`sqlite_sql_translate.py`'s 20 surviving mutants, closed** (#265). This module (`_translate_sql`/`_returning_was_stripped`, split out of `sqlite_compat.py` by #260) left 20 mutants surviving a scoped mutmut run — every one the same shape: a mutant dropping (or re-spelling the case of) the `flags=re.IGNORECASE` argument on one of the module's `re.sub`/`re.search` calls. Every existing case-insensitivity fixture supplies an input whose case already matches the pattern's own literal spelling, so the flag's presence was never observable. A rescoped run on this tree measured 19 of the 20 named ids still surviving (`mutmut_136` had flipped to killed between runs — non-deterministic mutant/worker ordering, not a real fix, folded back into the equivalent set below by direct regex comparison). Six are real gaps, closed with `tests_py/infrastructure/test_sqlite_sql_translate_265.py` supplying the opposite-case input for each: lowercase `DEFAULT now()`, an uppercase `&&`-overlap column, `XMAX`/`as` case variants on the xmax-drop rule, a lowercase `RETURNING` strip inside `_translate_sql` distinct from the one `_returning_was_stripped` already covered, an uppercase `ARRAY_LENGTH`, and lowercase `_returning_was_stripped` input under a monkeypatched `_SUPPORTS_RETURNING` — 6 new tests, all failing on pre-fix code. The remaining 14 are **documented equivalent mutants**: the mutation only re-spells the pattern's own literal case (`SERIAL`→`serial`, char classes `[a-z_]`↔`[A-Z_]`, etc.) while `re.IGNORECASE` stays in place, which Python's `re` semantics make provably irrelevant to the match — confirmed empirically with a differential harness (uppercase/lowercase/mixed-case probes against the original and mutated pattern, identical match results in every case) rather than asserted by inspection alone. Re-running the reproduction after the fix: 6 killed, 14 equivalent, 0 unaccounted-for survivors. - **`mcp-toplist-badge.yml`'s monthly refresh can now actually open its PR** (#273). A real `workflow_dispatch` run (triggered while dispatch-verifying #246) reached `Open refresh PR` and failed there: `GitHub Actions is not permitted to create or approve pull requests`. The repo had **"Allow GitHub Actions to create and approve pull requests"** unchecked at Settings → Actions → General, which blocks PR *creation* itself — a stronger failure than the one the workflow's own comment anticipated ("GitHub deliberately does not trigger workflows on `GITHUB_TOKEN`-authored PRs", which only explains why such a PR's checks don't start, not why it would fail to be created at all). Fixed at the repo-policy layer (`gh api -X PUT repos/cdeust/Cortex/actions/permissions/workflow -F can_approve_pull_request_reviews=true`), which is where the root cause lives — not in the workflow, which already had the correct `secrets.BADGE_REFRESH_TOKEN || secrets.GITHUB_TOKEN` fallback and needed no logic change. The workflow's comment now documents both distinct failure modes and the fix, so a future repo transfer or org policy reset that reintroduces this is diagnosable from the file alone. A stray `chore/mcp-toplist-badge-refresh` branch pushed-then-abandoned by the earlier failing run had already been deleted (confirmed absent by this fix); the verification run for this fix leaves no stray branch either — its outcome is quoted in PR #273's description. - **`sqlite_compat.py` relied on sqlite3's *implicit* default `datetime` adapter, deprecated as of Python 3.12** (#260), firing on 3 tests (`test_consolidate.py::test_with_memories`, `::test_protected_memories_skip_compression`, `test_memory_lifecycle.py::test_store_consolidate_recall`). Root cause: `cascade.py::_update_stage_entered` binds a raw `datetime.datetime` object as a SQL parameter instead of an ISO string — confirmed the **sole** such call site in this codebase by instrumenting all three `execute`/`executemany` paths in `sqlite_compat.py` and running the full suite against it. Fix: an explicit `sqlite3.register_adapter(datetime, _adapt_datetime_iso)` (the sanctioned Python-docs recipe the deprecation warning itself points to), writing the same "T"-separated `.isoformat()` spelling every other datetime write path here already produces (`sqlite_store._now_iso()`, etc.) — one canonical wire format instead of two. Old rows on disk (the deprecated adapter's space-separated spelling) keep reading correctly: `datetime.fromisoformat()` — the read path every consumer here uses — parses both spellings to an identical value (verified empirically and pinned by a test), so **no migration is required**. A `pyproject.toml` `filterwarnings` entry turns this specific DeprecationWarning into a hard failure going forward — a regression tripwire, not a silence. Boy-scout: `sqlite_compat.py` was already 335 lines — over this repo's 300-line file cap — before this change touched it; split the pure SQL-dialect translation logic (`_translate_sql`/`_returning_was_stripped`/`_SUPPORTS_RETURNING`) into a new `sqlite_sql_translate.py` module (behaviour-preserving, byte-identical logic), retargeting the two existing tests that monkeypatched `_SUPPORTS_RETURNING` to the module that actually defines it. Scoped mutation testing (mutmut) against the touched `sqlite_compat.py` surfaced 13 pre-existing gaps in `_CompatCursor`/`_CompatExecutingCursor`/`PsycopgCompatConnection` field wiring (not the datetime fix itself) — added targeted tests for all of them; one mutant (`executemany`'s `self.lastrowid = None`) is a documented equivalent (`sqlite3.Cursor.lastrowid` is only meaningful after a single-row `execute()` INSERT, so it is always `None` after `executemany()` regardless of which literal is written). A separate mutation gap in `_translate_sql` itself (20 survivors, verbatim regex-translation code moved unchanged from `sqlite_compat.py`, same tests before/after) predates this change and is filed as #265 rather than folded in, per §14.3. +- **The typecheck gate's zero-diagnostic verdict was a property of the pip resolver, not of the source** (#249). `mcp_server/core/ast_parser.py:88` called `tree_sitter_language_pack.get_parser(language)` with a plain `str`; `get_parser`'s declared parameter type differs across the package's own releases — a `SupportedLanguage` `Literal` union in 1.6.2 (what `uv.lock` resolved when this was filed) versus a plain `str` in 1.13.5 (what both `uv.lock` and a fresh pip resolution give today) — so the same call was a pyright error under one and silently clean under the other, and a `set[str]` `in` test narrowed nothing either way. `AST_SUPPORTED` is now a `Literal` union (`_SupportedLanguage`) narrowed via a `TypeGuard` (`_is_ast_supported`) rather than a bare `set[str]`; reproduced against both a live 1.6.2 install and the current 1.13.5 one (`pyright mcp_server/`: 1 error → 0 under 1.13.5; the 1.6.2 stub's own `SupportedLanguage` omits `"csharp"` outright — a pre-existing grammar-availability gap in that release, not a narrowing defect, and unreachable under any version this repo's floor resolves to today). The typecheck CI job now quotes the resolved `tree-sitter-language-pack` version in its log, since that resolution is now load-bearing evidence for the verdict, not incidental (the job's environment reproducibility and its `uv.lock`-drift guard were already closed by #244's hash-pinned `requirements/ci-typecheck.txt`). Boy-scout pass on the touched file: the flat `calls` list every per-language extractor computed via `extract_calls_generic` and `parse_file_ast` immediately discarded (superseded by `calls_per_function`, never itself consumed) is removed rather than given a test; Swift and Rust parsing had zero test coverage through `parse_file_ast` (`_extract_swift`/`_extract_rust` were unreachable from any existing test — the Go equivalent bypassed its own wrapper by calling `extract_go_definitions` directly) and now do, alongside coverage gaps in `is_available`, `_node_text`, `_extract_module_doc`'s docstring/comment branches, `content_hash` length, and `calls_per_function`'s populated content. Suite grows 6526 → 6549. - **A `created_at` that states a timezone was stored as the wrong instant** (#252). `normalize_date_to_iso` had no timezone policy on any of its paths, so three defects stacked: (1) the "already ISO" guard was the substring test `"T" in raw`, and every US zone abbreviation contains a T — `8 May 2023 13:56 EST` was returned unparsed; (2) the built-in fast path matches the date at the START of the string and discards the rest, so `8 May 2023 13:56 +02:00` became midnight, dropping both the time and the offset; (3) on the dateutil path an abbreviation it cannot resolve is dropped with a warning nobody sees, leaving a naive datetime that PostgreSQL's `timestamptz` cast and `compute_recency_boost` both read as UTC. The instant was up to a day off and nothing was emitted. A stated zone is now honoured or the value is refused: `mcp_server/core/temporal_timezones.py` supplies dateutil a `tzinfos` resolver over the **RFC 5322 §4.3 obs-zone table** (the normative answer to "which EST?" — cross-checked against CPython's `email._parseaddr._timezones`), and any abbreviation outside it is refused with a warning naming the input, the abbreviation and the fix, rather than defaulted. Parsing no longer depends on the host's local zone name, and no `warnings.catch_warnings()` — process-global and not thread-safe — is taken on a store write path. `normalize_date_to_iso` moves out of `core/temporal.py` into `core/temporal_normalize.py`: storage normalization must not lose precision, retrieval scoring may, and they are now separate modules (both stores import from the new path). The refusal also covers the degraded path — a string that states a zone is never salvaged to a naive date, whatever made the parse fail. - **`python-dateutil` is now a declared dependency** (#252). `normalize_date_to_iso` has always parsed free-form dates with a time of day through it — the LoCoMo shape `1:56 pm on 8 May, 2023` its own docstring cites — but it was never declared and arrived only by transitive luck. It was absent from every `requirements/ci-*.txt`, so **every CI test job ran with that parser missing**: the fallback was dead code in CI, and any install resolving without it kept dates it could not read. Declared, locked and hash-pinned into the 9 exported requirement sets, so the write path behaves identically on every install and both backends. - **Both stores skipped the normalization entirely for the dates that needed it most** (#252, the same substring defect one layer up). `PgMemoryStore._build_insert_params` and `SqliteMemoryStore._insert_memory_rows` each guarded the call with `"T" not in raw_created` as a cheap "is it already ISO?" test — so `8 May 2023 13:56 EST` (and every `PST`/`CST`/`MST` string) went to the database untouched, whichever way `normalize_date_to_iso` behaved. The guard is gone from both: deciding what is already ISO belongs to the function that owns it, which returns a real ISO datetime unchanged. Asserted on the stored row, not just on the parser, and asserted equal across the two backends. diff --git a/mcp_server/core/ast_extractor_registry.py b/mcp_server/core/ast_extractor_registry.py index 287af6b7..f20a420a 100644 --- a/mcp_server/core/ast_extractor_registry.py +++ b/mcp_server/core/ast_extractor_registry.py @@ -1,6 +1,6 @@ """Registry for the extra-language tree-sitter extractors. -Builds the (imports, definitions, calls) extractor callables for the JVM, +Builds the (imports, definitions) extractor callables for the JVM, C-family, and scripting language groups and merges them into one dict for ast_parser._EXTRACTORS. Split out so ast_parser.py stays under 300 lines. @@ -12,7 +12,6 @@ from typing import TYPE_CHECKING, Callable from mcp_server.core.codebase_parser import ImportInfo, SymbolDef -from mcp_server.core.ast_extractors import extract_calls_generic from mcp_server.core.ast_extractors_clike import ( extract_c_definitions, extract_c_imports, @@ -37,27 +36,27 @@ from tree_sitter import Node from tree_sitter_language_pack import SupportedLanguage -Extractor = Callable[ - ["Node", bytes], tuple[list[ImportInfo], list[SymbolDef], list[str]] -] +Extractor = Callable[["Node", bytes], tuple[list[ImportInfo], list[SymbolDef]]] def _make_extractor( imports_fn: Callable[["Node", bytes], list[ImportInfo]], defs_fn: Callable[["Node", bytes], list[SymbolDef]], ) -> Extractor: - """Compose an extractor from an imports fn, a defs fn, and generic calls.""" - - def _extract( - root: Node, - source: bytes, - ) -> tuple[list[ImportInfo], list[SymbolDef], list[str]]: - - return ( - imports_fn(root, source), - defs_fn(root, source), - extract_calls_generic(root, source), - ) + """Compose an extractor from an imports fn and a defs fn. + + Precondition: `imports_fn`/`defs_fn` are pure (no I/O), taking the same + `(root, source)` pair. + Postcondition: returns a callable producing `(imports, definitions)` — + the flat per-file call list this used to also return + (`extract_calls_generic(root, source)`) was computed and then discarded + by every caller (`parse_file_ast` only reads the `calls_per_function` + map, populated separately via `extract_calls_per_function`); removed as + dead code (issue #249 boy-scout pass) rather than given a test. + """ + + def _extract(root: Node, source: bytes) -> tuple[list[ImportInfo], list[SymbolDef]]: + return imports_fn(root, source), defs_fn(root, source) return _extract diff --git a/mcp_server/core/ast_parser.py b/mcp_server/core/ast_parser.py index a1c5c91d..598c5e9e 100644 --- a/mcp_server/core/ast_parser.py +++ b/mcp_server/core/ast_parser.py @@ -27,7 +27,6 @@ from mcp_server.core.codebase_parser import parse_file from mcp_server.core.ast_extractors import ( extract_calls_per_function, - extract_calls_generic, extract_python_definitions, extract_python_imports, extract_js_definitions, @@ -101,7 +100,7 @@ def parse_file_ast(path: str, content: bytes) -> FileAnalysis: return parse_file(path, text) extractor, tree = result - imports, definitions, calls = extractor(tree.root_node, content) + imports, definitions = extractor(tree.root_node, content) docstring = _extract_module_doc(tree.root_node, language, content) # Caller-qualified call map — works across every language the # extractor covers because it targets tree-sitter node types shared @@ -156,13 +155,12 @@ def _extract_module_doc( def _extract_python( root: Node, source: bytes, -) -> tuple[list[ImportInfo], list[SymbolDef], list[str]]: - """Extract Python imports, definitions, and call sites.""" +) -> tuple[list[ImportInfo], list[SymbolDef]]: + """Extract Python imports and definitions.""" imports = extract_python_imports(root, source) definitions = extract_python_definitions(root, source) - calls = extract_calls_generic(root, source) - return imports, definitions, calls + return imports, definitions # ── JS/TS extractor ────────────────────────────────────────────────────── @@ -171,13 +169,10 @@ def _extract_python( def _extract_js( root: Node, source: bytes, -) -> tuple[list[ImportInfo], list[SymbolDef], list[str]]: - """Extract JavaScript/TypeScript imports, definitions, and calls.""" +) -> tuple[list[ImportInfo], list[SymbolDef]]: + """Extract JavaScript/TypeScript imports and definitions.""" - imports = extract_js_imports(root, source) - definitions = extract_js_definitions(root, source) - calls = extract_calls_generic(root, source) - return imports, definitions, calls + return extract_js_imports(root, source), extract_js_definitions(root, source) # ── Go extractor ───────────────────────────────────────────────────────── @@ -186,40 +181,28 @@ def _extract_js( def _extract_go( root: Node, source: bytes, -) -> tuple[list[ImportInfo], list[SymbolDef], list[str]]: - """Extract Go imports, definitions, and calls.""" +) -> tuple[list[ImportInfo], list[SymbolDef]]: + """Extract Go imports and definitions.""" - return ( - extract_go_imports(root, source), - extract_go_definitions(root, source), - extract_calls_generic(root, source), - ) + return extract_go_imports(root, source), extract_go_definitions(root, source) def _extract_swift( root: Node, source: bytes, -) -> tuple[list[ImportInfo], list[SymbolDef], list[str]]: - """Extract Swift imports, definitions, and calls.""" +) -> tuple[list[ImportInfo], list[SymbolDef]]: + """Extract Swift imports and definitions.""" - return ( - extract_swift_imports(root, source), - extract_swift_definitions(root, source), - extract_calls_generic(root, source), - ) + return extract_swift_imports(root, source), extract_swift_definitions(root, source) def _extract_rust( root: Node, source: bytes, -) -> tuple[list[ImportInfo], list[SymbolDef], list[str]]: - """Extract Rust imports, definitions, and calls.""" +) -> tuple[list[ImportInfo], list[SymbolDef]]: + """Extract Rust imports and definitions.""" - return ( - extract_rust_imports(root, source), - extract_rust_definitions(root, source), - extract_calls_generic(root, source), - ) + return extract_rust_imports(root, source), extract_rust_definitions(root, source) # Keyed by the language pack's own `SupportedLanguage` literal, not by `str`: diff --git a/tests_py/core/test_ast_parser.py b/tests_py/core/test_ast_parser.py index 1bd69fee..4fcecd57 100644 --- a/tests_py/core/test_ast_parser.py +++ b/tests_py/core/test_ast_parser.py @@ -7,11 +7,41 @@ from __future__ import annotations -from mcp_server.core.ast_parser import is_available, parse_file_ast +import sys + +import pytest + +from mcp_server.core.ast_parser import ( + _extract_module_doc, + _node_text, + is_available, + parse_file_ast, +) _HAS_TREE_SITTER = is_available() +class _FakeNode: + """Minimal duck-typed stand-in for a tree_sitter.Node. + + ast_parser's docstring/comment extraction only reads `.type`, + `.children`, `.start_byte`, `.end_byte` — a real grammar is not needed + to pin these functions' branches precisely. + """ + + def __init__( + self, + node_type: str, + children: list["_FakeNode"] | None = None, + start_byte: int = 0, + end_byte: int = 0, + ) -> None: + self.type = node_type + self.children = children or [] + self.start_byte = start_byte + self.end_byte = end_byte + + class TestParseFilePython: SAMPLE = b'''"""Auth middleware module.""" @@ -107,6 +137,107 @@ def test_fallback_returns_valid_analysis(self) -> None: assert r.language == "python" assert r.line_count >= 2 + def test_content_hash_length(self) -> None: + r = parse_file_ast("a.py", b"def foo(): pass") + assert len(r.content_hash) == 16 + + def test_calls_per_function_populated(self) -> None: + r = parse_file_ast("auth/middleware.py", self.SAMPLE) + if _HAS_TREE_SITTER: + assert "AuthMiddleware.authenticate" in r.calls_per_function + assert "verify_jwt" in r.calls_per_function["AuthMiddleware.authenticate"] + else: + assert r.calls_per_function == {} + + def test_decodes_malformed_utf8_without_raising(self) -> None: + """`errors="replace"` must survive a resolver swap, per issue #249.""" + r = parse_file_ast("bad.py", b"\xff\xfe def foo(): pass") + assert isinstance(r.line_count, int) + + +def test_is_available_reflects_installed_tree_sitter() -> None: + """Pins is_available()'s True branch — a mutant flipping it to False + would still leave every _HAS_TREE_SITTER-gated assertion above passing + trivially (the gated branch is simply skipped).""" + if not _HAS_TREE_SITTER: + pytest.skip("tree-sitter not installed") + assert is_available() is True + + +def test_is_available_false_when_tree_sitter_import_fails( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Pins the except-ImportError branch, unreachable when tree-sitter IS + installed (this environment): forcing the import to fail is the only + way to exercise it directly.""" + monkeypatch.setitem(sys.modules, "tree_sitter_language_pack", None) + assert is_available() is False + + +class TestExtractModuleDoc: + """Direct unit tests for the docstring/comment-extraction branches — + fake nodes pin exact behavior without depending on a specific grammar's + tree shape (issue #249 mutation-testing pass).""" + + def test_no_children_returns_empty(self) -> None: + root = _FakeNode("module", children=[]) + assert _extract_module_doc(root, "python", b"") == "" + + def test_python_bare_string_first_child(self) -> None: + source = b'"""hello"""' + string_node = _FakeNode("string", start_byte=0, end_byte=len(source)) + root = _FakeNode("module", children=[string_node]) + assert _extract_module_doc(root, "python", source) == "hello" + + def test_python_expression_statement_wrapper(self) -> None: + source = b'"""Xylophone is great."""' + string_node = _FakeNode("string", start_byte=0, end_byte=len(source)) + other = _FakeNode("other") + expr_stmt = _FakeNode("expression_statement", children=[string_node, other]) + root = _FakeNode("module", children=[expr_stmt]) + assert _extract_module_doc(root, "python", source) == "Xylophone is great." + + def test_python_docstring_truncated_at_200(self) -> None: + body = "x" * 250 + source = f'"""{body}"""'.encode() + string_node = _FakeNode("string", start_byte=0, end_byte=len(source)) + root = _FakeNode("module", children=[string_node]) + doc = _extract_module_doc(root, "python", source) + assert len(doc) == 200 + + def test_non_python_comment_first_child(self) -> None: + source = b"# Xray diagnostics." + comment = _FakeNode("comment", start_byte=0, end_byte=len(source)) + root = _FakeNode("module", children=[comment]) + assert _extract_module_doc(root, "javascript", source) == "Xray diagnostics." + + def test_comment_truncated_at_200(self) -> None: + source = ("# " + "y" * 250).encode() + comment = _FakeNode("comment", start_byte=0, end_byte=len(source)) + root = _FakeNode("module", children=[comment]) + doc = _extract_module_doc(root, "javascript", source) + assert len(doc) == 200 + + def test_neither_string_nor_comment_returns_empty(self) -> None: + root = _FakeNode("module", children=[_FakeNode("class_declaration")]) + assert _extract_module_doc(root, "go", b"") == "" + + +class TestNodeText: + def test_extracts_slice_as_utf8(self) -> None: + source = b"hello world" + node = _FakeNode("identifier", start_byte=0, end_byte=5) + assert _node_text(node, source) == "hello" + + def test_replaces_invalid_utf8_instead_of_raising(self) -> None: + """`errors="replace"` must survive a resolver swap, per issue #249: + an invalid error-handler name (a stub/version mismatch) raises + LookupError here instead of degrading gracefully.""" + source = b"\xff\xfe" + node = _FakeNode("raw", start_byte=0, end_byte=len(source)) + text = _node_text(node, source) + assert isinstance(text, str) + class TestParseFileTypeScript: SAMPLE = b"""import { Request } from 'express'; @@ -169,3 +300,106 @@ def test_unknown_extension_uses_regex(self) -> None: r = parse_file_ast("readme.md", b"# Hello") assert r.language == "unknown" assert r.definitions == [] + + +class TestParseFileGo: + """Exercises ast_parser._extract_go through the public parse_file_ast + entry point — TestGoExtractors (test_ast_extractors.py) calls + extract_go_definitions directly and never reaches this wrapper + (issue #249 mutation-testing pass).""" + + SAMPLE = b"""package main + +import "fmt" + +type Server struct { + port int +} + +func (s *Server) Start() error { + return nil +} + +func NewServer(port int) *Server { + return &Server{port: port} +} +""" + + def test_returns_file_analysis(self) -> None: + r = parse_file_ast("main.go", self.SAMPLE) + assert r.language == "go" + + def test_imports(self) -> None: + r = parse_file_ast("main.go", self.SAMPLE) + if _HAS_TREE_SITTER: + assert "fmt" in [i.module for i in r.imports] + + def test_definitions(self) -> None: + r = parse_file_ast("main.go", self.SAMPLE) + if _HAS_TREE_SITTER: + names = [d.name for d in r.definitions] + assert "Server.Start" in names + assert "NewServer" in names + + +class TestParseFileSwift: + """Exercises ast_parser._extract_swift through parse_file_ast — no + prior test reached this wrapper at all (issue #249 mutation-testing + pass: 12/12 mutants reported "no tests" before this class).""" + + SAMPLE = b"""import Foundation + +class AuthService { + func verify(token: String) -> Bool { + return true + } +} +""" + + def test_returns_file_analysis(self) -> None: + r = parse_file_ast("Auth.swift", self.SAMPLE) + assert r.language == "swift" + + def test_imports(self) -> None: + r = parse_file_ast("Auth.swift", self.SAMPLE) + if _HAS_TREE_SITTER: + assert "Foundation" in [i.module for i in r.imports] + + def test_definitions(self) -> None: + r = parse_file_ast("Auth.swift", self.SAMPLE) + if _HAS_TREE_SITTER: + names = [d.name for d in r.definitions] + assert "AuthService" in names + + +class TestParseFileRust: + """Exercises ast_parser._extract_rust through parse_file_ast — no + prior test reached this wrapper at all (issue #249 mutation-testing + pass: 12/12 mutants reported "no tests" before this class).""" + + SAMPLE = b"""use std::fmt; + +struct Server { + port: u16, +} + +fn new_server(port: u16) -> Server { + Server { port } +} +""" + + def test_returns_file_analysis(self) -> None: + r = parse_file_ast("main.rs", self.SAMPLE) + assert r.language == "rust" + + def test_imports(self) -> None: + r = parse_file_ast("main.rs", self.SAMPLE) + if _HAS_TREE_SITTER: + assert "std::fmt" in [i.module for i in r.imports] + + def test_definitions(self) -> None: + r = parse_file_ast("main.rs", self.SAMPLE) + if _HAS_TREE_SITTER: + names = [d.name for d in r.definitions] + assert "Server" in names + assert "new_server" in names From b8fb34494c37200251588c007cb4957fe36c59c3 Mon Sep 17 00:00:00 2001 From: cdeust Date: Thu, 30 Jul 2026 00:45:22 +0200 Subject: [PATCH 2/6] =?UTF-8?q?chore:=20reconcile=20rebase=20onto=20main?= =?UTF-8?q?=20(#261)=20=E2=80=94=20recompute=20suite=20count,=20fix=20CHAN?= =?UTF-8?q?GELOG=20claim?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebasing onto main pulled in #261/#253/#251, which independently closed via the same TypeGuard-narrowing mechanism, already keyed off the language pack's own SupportedLanguage type and with AST_SUPPORTED derived from _EXTRACTORS rather than restated. This branch's own duplicate _SupportedLanguage/_is_ast_supported/AST_SUPPORTED-via-get_args mechanism is dropped in favour of main's (ast_parser.py, ast_extractor_registry.py); this branch's actual surviving contribution — removing the dead `calls` tuple element from every extractor and adding the missing Swift/Rust/ is_available/_node_text/_extract_module_doc/content_hash/calls_per_function test coverage — is preserved and now sits on top of main's mechanism cleanly (auto-merged with no conflicts in ast_extractor_registry.py). CHANGELOG's #249 entry rewritten: it no longer claims to close #249 (already closed by #261) and instead describes only the boy-scout dead-code-removal and test-coverage work this branch still adds. Suite count recomputed from a live `pytest --collect-only -q` on this machine (6572, up from main's 6549 by the 23 tests this branch adds) per the standing rule that the count is a measured absolute, not a delta from an advertised number: CLAUDE.md, README.md, CONTRIBUTING.md, .bestpractices.json, docs/ASSURANCE-CASE.md, assets/badge-tests.svg. Verified: scripts/check_doc_claims.py --test-count 6572 and scripts/generate_repo_badges.py --check --test-count 6572 both green; tests_py/scripts/test_typecheck_env_parity.py, test_check_doc_claims.py, test_generate_repo_badges.py all pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e83ff7a..e88a3753 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ adheres to [Semantic Versioning](https://semver.org/). - **`mcp-toplist-badge.yml`'s monthly refresh can now actually open its PR** (#273). A real `workflow_dispatch` run (triggered while dispatch-verifying #246) reached `Open refresh PR` and failed there: `GitHub Actions is not permitted to create or approve pull requests`. The repo had **"Allow GitHub Actions to create and approve pull requests"** unchecked at Settings → Actions → General, which blocks PR *creation* itself — a stronger failure than the one the workflow's own comment anticipated ("GitHub deliberately does not trigger workflows on `GITHUB_TOKEN`-authored PRs", which only explains why such a PR's checks don't start, not why it would fail to be created at all). Fixed at the repo-policy layer (`gh api -X PUT repos/cdeust/Cortex/actions/permissions/workflow -F can_approve_pull_request_reviews=true`), which is where the root cause lives — not in the workflow, which already had the correct `secrets.BADGE_REFRESH_TOKEN || secrets.GITHUB_TOKEN` fallback and needed no logic change. The workflow's comment now documents both distinct failure modes and the fix, so a future repo transfer or org policy reset that reintroduces this is diagnosable from the file alone. A stray `chore/mcp-toplist-badge-refresh` branch pushed-then-abandoned by the earlier failing run had already been deleted (confirmed absent by this fix); the verification run for this fix leaves no stray branch either — its outcome is quoted in PR #273's description. - **`sqlite_compat.py` relied on sqlite3's *implicit* default `datetime` adapter, deprecated as of Python 3.12** (#260), firing on 3 tests (`test_consolidate.py::test_with_memories`, `::test_protected_memories_skip_compression`, `test_memory_lifecycle.py::test_store_consolidate_recall`). Root cause: `cascade.py::_update_stage_entered` binds a raw `datetime.datetime` object as a SQL parameter instead of an ISO string — confirmed the **sole** such call site in this codebase by instrumenting all three `execute`/`executemany` paths in `sqlite_compat.py` and running the full suite against it. Fix: an explicit `sqlite3.register_adapter(datetime, _adapt_datetime_iso)` (the sanctioned Python-docs recipe the deprecation warning itself points to), writing the same "T"-separated `.isoformat()` spelling every other datetime write path here already produces (`sqlite_store._now_iso()`, etc.) — one canonical wire format instead of two. Old rows on disk (the deprecated adapter's space-separated spelling) keep reading correctly: `datetime.fromisoformat()` — the read path every consumer here uses — parses both spellings to an identical value (verified empirically and pinned by a test), so **no migration is required**. A `pyproject.toml` `filterwarnings` entry turns this specific DeprecationWarning into a hard failure going forward — a regression tripwire, not a silence. Boy-scout: `sqlite_compat.py` was already 335 lines — over this repo's 300-line file cap — before this change touched it; split the pure SQL-dialect translation logic (`_translate_sql`/`_returning_was_stripped`/`_SUPPORTS_RETURNING`) into a new `sqlite_sql_translate.py` module (behaviour-preserving, byte-identical logic), retargeting the two existing tests that monkeypatched `_SUPPORTS_RETURNING` to the module that actually defines it. Scoped mutation testing (mutmut) against the touched `sqlite_compat.py` surfaced 13 pre-existing gaps in `_CompatCursor`/`_CompatExecutingCursor`/`PsycopgCompatConnection` field wiring (not the datetime fix itself) — added targeted tests for all of them; one mutant (`executemany`'s `self.lastrowid = None`) is a documented equivalent (`sqlite3.Cursor.lastrowid` is only meaningful after a single-row `execute()` INSERT, so it is always `None` after `executemany()` regardless of which literal is written). A separate mutation gap in `_translate_sql` itself (20 survivors, verbatim regex-translation code moved unchanged from `sqlite_compat.py`, same tests before/after) predates this change and is filed as #265 rather than folded in, per §14.3. - **The typecheck gate's zero-diagnostic verdict was a property of the pip resolver, not of the source** (#249). `mcp_server/core/ast_parser.py:88` called `tree_sitter_language_pack.get_parser(language)` with a plain `str`; `get_parser`'s declared parameter type differs across the package's own releases — a `SupportedLanguage` `Literal` union in 1.6.2 (what `uv.lock` resolved when this was filed) versus a plain `str` in 1.13.5 (what both `uv.lock` and a fresh pip resolution give today) — so the same call was a pyright error under one and silently clean under the other, and a `set[str]` `in` test narrowed nothing either way. `AST_SUPPORTED` is now a `Literal` union (`_SupportedLanguage`) narrowed via a `TypeGuard` (`_is_ast_supported`) rather than a bare `set[str]`; reproduced against both a live 1.6.2 install and the current 1.13.5 one (`pyright mcp_server/`: 1 error → 0 under 1.13.5; the 1.6.2 stub's own `SupportedLanguage` omits `"csharp"` outright — a pre-existing grammar-availability gap in that release, not a narrowing defect, and unreachable under any version this repo's floor resolves to today). The typecheck CI job now quotes the resolved `tree-sitter-language-pack` version in its log, since that resolution is now load-bearing evidence for the verdict, not incidental (the job's environment reproducibility and its `uv.lock`-drift guard were already closed by #244's hash-pinned `requirements/ci-typecheck.txt`). Boy-scout pass on the touched file: the flat `calls` list every per-language extractor computed via `extract_calls_generic` and `parse_file_ast` immediately discarded (superseded by `calls_per_function`, never itself consumed) is removed rather than given a test; Swift and Rust parsing had zero test coverage through `parse_file_ast` (`_extract_swift`/`_extract_rust` were unreachable from any existing test — the Go equivalent bypassed its own wrapper by calling `extract_go_definitions` directly) and now do, alongside coverage gaps in `is_available`, `_node_text`, `_extract_module_doc`'s docstring/comment branches, `content_hash` length, and `calls_per_function`'s populated content. Suite grows 6526 → 6549. +- **`ast_parser`'s per-language extractors carried a dead return value, and two grammars had zero test coverage through their real call path** (boy-scout follow-on from #249; the resolver-dependent typecheck defect #249 itself named was independently closed by #253/#251's `_is_ast_language`/`SupportedLanguage`-derived-`AST_SUPPORTED` fix, already on `main`). Every per-language extractor (`_extract_python`/`_extract_js`/`_extract_go`/`_extract_swift`/`_extract_rust`, plus the JVM/C-family/scripting extractors `ast_extractor_registry.build_extra_extractors` composes) computed a flat `calls` list via `extract_calls_generic` and returned it as a 3rd tuple element that `parse_file_ast` immediately discarded — superseded by `calls_per_function`, the value actually consumed downstream. Removed; `Extractor` is now a 2-tuple `(imports, definitions)`. Swift and Rust parsing had never been exercised through `parse_file_ast` by any existing test (the Go equivalent bypassed its own wrapper by calling `extract_go_definitions` directly) — added `TestParseFileGo`/`TestParseFileSwift`/`TestParseFileRust`. Smaller gaps closed alongside: both branches of `is_available()`, the docstring/comment-extraction branches of `_node_text`/`_extract_module_doc` (via fake-`Node` unit tests), `content_hash`'s length invariant, `calls_per_function`'s populated content, and malformed-UTF-8 decode robustness (`errors="replace"`). Suite grows 6550 → 6573 (measured via `pytest --collect-only -q` against this repo's shared venv, matching CI's canonical collection per the parity this repo's `test_typecheck_env_parity.py`/`check_doc_claims.py` gates already establish). - **A `created_at` that states a timezone was stored as the wrong instant** (#252). `normalize_date_to_iso` had no timezone policy on any of its paths, so three defects stacked: (1) the "already ISO" guard was the substring test `"T" in raw`, and every US zone abbreviation contains a T — `8 May 2023 13:56 EST` was returned unparsed; (2) the built-in fast path matches the date at the START of the string and discards the rest, so `8 May 2023 13:56 +02:00` became midnight, dropping both the time and the offset; (3) on the dateutil path an abbreviation it cannot resolve is dropped with a warning nobody sees, leaving a naive datetime that PostgreSQL's `timestamptz` cast and `compute_recency_boost` both read as UTC. The instant was up to a day off and nothing was emitted. A stated zone is now honoured or the value is refused: `mcp_server/core/temporal_timezones.py` supplies dateutil a `tzinfos` resolver over the **RFC 5322 §4.3 obs-zone table** (the normative answer to "which EST?" — cross-checked against CPython's `email._parseaddr._timezones`), and any abbreviation outside it is refused with a warning naming the input, the abbreviation and the fix, rather than defaulted. Parsing no longer depends on the host's local zone name, and no `warnings.catch_warnings()` — process-global and not thread-safe — is taken on a store write path. `normalize_date_to_iso` moves out of `core/temporal.py` into `core/temporal_normalize.py`: storage normalization must not lose precision, retrieval scoring may, and they are now separate modules (both stores import from the new path). The refusal also covers the degraded path — a string that states a zone is never salvaged to a naive date, whatever made the parse fail. - **`python-dateutil` is now a declared dependency** (#252). `normalize_date_to_iso` has always parsed free-form dates with a time of day through it — the LoCoMo shape `1:56 pm on 8 May, 2023` its own docstring cites — but it was never declared and arrived only by transitive luck. It was absent from every `requirements/ci-*.txt`, so **every CI test job ran with that parser missing**: the fallback was dead code in CI, and any install resolving without it kept dates it could not read. Declared, locked and hash-pinned into the 9 exported requirement sets, so the write path behaves identically on every install and both backends. - **Both stores skipped the normalization entirely for the dates that needed it most** (#252, the same substring defect one layer up). `PgMemoryStore._build_insert_params` and `SqliteMemoryStore._insert_memory_rows` each guarded the call with `"T" not in raw_created` as a cheap "is it already ISO?" test — so `8 May 2023 13:56 EST` (and every `PST`/`CST`/`MST` string) went to the database untouched, whichever way `normalize_date_to_iso` behaved. The guard is gone from both: deciding what is already ISO belongs to the function that owns it, which returns a real ISO datetime unchanged. Asserted on the stored row, not just on the parser, and asserted equal across the two backends. From 3b822933dcfbffac8ac674ddd5d8e8214d0900f9 Mon Sep 17 00:00:00 2001 From: cdeust Date: Thu, 30 Jul 2026 03:20:33 +0200 Subject: [PATCH 3/6] =?UTF-8?q?chore(ast):=20delete=20extract=5Fcalls=5Fge?= =?UTF-8?q?neric=20=E2=80=94=20its=20own=20direct=20unit=20test=20was=20th?= =?UTF-8?q?e=20last=20caller=20(#249=20review)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding on PR #270: removing the dead `calls` tuple element from every per-language extractor (previous commit in this branch) stranded extract_calls_generic (mcp_server/core/ast_extractors.py) — nothing in production called it anymore, only TestCallExtraction's two tests called it directly. Dead code per coding-standards.md §9: deleted the function and its now-pointless direct unit test, corrected ast_extractors.py's module docstring (dropped the false "Also provides the generic call-site extractor used by all languages" claim), and updated ast_extractor_registry.py's _make_extractor docstring to record that the function itself, not just its result, is now gone. _MAX_CALL_NAME_LEN and _walk_type remain live — both are still used by the per-function, caller-qualified call extraction a few lines down (extract_calls_per_function), which is the value parse_file_ast actually consumes. Suite count recomputed from a live `pytest --collect-only -q` on this tree (6571, down 2 from 6573 for the deleted tests) and resynced across CLAUDE.md/README.md/CONTRIBUTING.md/.bestpractices.json/ docs/ASSURANCE-CASE.md/assets/badge-tests.svg; scripts/check_doc_claims.py --test-count 6571 and scripts/generate_repo_badges.py --check --test-count 6571 both green. Verification: tests_py/core/test_ast_parser.py + test_ast_extractors_multilang.py + test_ast_extractors.py + test_ast_parser_language_contract.py (69 passed), tests_py/core/ (3384 passed), full suite (6571 passed, 117 subtests passed, exact match to the recomputed collect-only count), ruff check + ruff format --check clean on all three changed .py files. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u --- CHANGELOG.md | 1 + mcp_server/core/ast_extractor_registry.py | 13 ++++---- mcp_server/core/ast_extractors.py | 20 ------------- tests_py/core/test_ast_extractors.py | 36 ----------------------- 4 files changed, 9 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e88a3753..49fa2531 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ adheres to [Semantic Versioning](https://semver.org/). - **`sqlite_compat.py` relied on sqlite3's *implicit* default `datetime` adapter, deprecated as of Python 3.12** (#260), firing on 3 tests (`test_consolidate.py::test_with_memories`, `::test_protected_memories_skip_compression`, `test_memory_lifecycle.py::test_store_consolidate_recall`). Root cause: `cascade.py::_update_stage_entered` binds a raw `datetime.datetime` object as a SQL parameter instead of an ISO string — confirmed the **sole** such call site in this codebase by instrumenting all three `execute`/`executemany` paths in `sqlite_compat.py` and running the full suite against it. Fix: an explicit `sqlite3.register_adapter(datetime, _adapt_datetime_iso)` (the sanctioned Python-docs recipe the deprecation warning itself points to), writing the same "T"-separated `.isoformat()` spelling every other datetime write path here already produces (`sqlite_store._now_iso()`, etc.) — one canonical wire format instead of two. Old rows on disk (the deprecated adapter's space-separated spelling) keep reading correctly: `datetime.fromisoformat()` — the read path every consumer here uses — parses both spellings to an identical value (verified empirically and pinned by a test), so **no migration is required**. A `pyproject.toml` `filterwarnings` entry turns this specific DeprecationWarning into a hard failure going forward — a regression tripwire, not a silence. Boy-scout: `sqlite_compat.py` was already 335 lines — over this repo's 300-line file cap — before this change touched it; split the pure SQL-dialect translation logic (`_translate_sql`/`_returning_was_stripped`/`_SUPPORTS_RETURNING`) into a new `sqlite_sql_translate.py` module (behaviour-preserving, byte-identical logic), retargeting the two existing tests that monkeypatched `_SUPPORTS_RETURNING` to the module that actually defines it. Scoped mutation testing (mutmut) against the touched `sqlite_compat.py` surfaced 13 pre-existing gaps in `_CompatCursor`/`_CompatExecutingCursor`/`PsycopgCompatConnection` field wiring (not the datetime fix itself) — added targeted tests for all of them; one mutant (`executemany`'s `self.lastrowid = None`) is a documented equivalent (`sqlite3.Cursor.lastrowid` is only meaningful after a single-row `execute()` INSERT, so it is always `None` after `executemany()` regardless of which literal is written). A separate mutation gap in `_translate_sql` itself (20 survivors, verbatim regex-translation code moved unchanged from `sqlite_compat.py`, same tests before/after) predates this change and is filed as #265 rather than folded in, per §14.3. - **The typecheck gate's zero-diagnostic verdict was a property of the pip resolver, not of the source** (#249). `mcp_server/core/ast_parser.py:88` called `tree_sitter_language_pack.get_parser(language)` with a plain `str`; `get_parser`'s declared parameter type differs across the package's own releases — a `SupportedLanguage` `Literal` union in 1.6.2 (what `uv.lock` resolved when this was filed) versus a plain `str` in 1.13.5 (what both `uv.lock` and a fresh pip resolution give today) — so the same call was a pyright error under one and silently clean under the other, and a `set[str]` `in` test narrowed nothing either way. `AST_SUPPORTED` is now a `Literal` union (`_SupportedLanguage`) narrowed via a `TypeGuard` (`_is_ast_supported`) rather than a bare `set[str]`; reproduced against both a live 1.6.2 install and the current 1.13.5 one (`pyright mcp_server/`: 1 error → 0 under 1.13.5; the 1.6.2 stub's own `SupportedLanguage` omits `"csharp"` outright — a pre-existing grammar-availability gap in that release, not a narrowing defect, and unreachable under any version this repo's floor resolves to today). The typecheck CI job now quotes the resolved `tree-sitter-language-pack` version in its log, since that resolution is now load-bearing evidence for the verdict, not incidental (the job's environment reproducibility and its `uv.lock`-drift guard were already closed by #244's hash-pinned `requirements/ci-typecheck.txt`). Boy-scout pass on the touched file: the flat `calls` list every per-language extractor computed via `extract_calls_generic` and `parse_file_ast` immediately discarded (superseded by `calls_per_function`, never itself consumed) is removed rather than given a test; Swift and Rust parsing had zero test coverage through `parse_file_ast` (`_extract_swift`/`_extract_rust` were unreachable from any existing test — the Go equivalent bypassed its own wrapper by calling `extract_go_definitions` directly) and now do, alongside coverage gaps in `is_available`, `_node_text`, `_extract_module_doc`'s docstring/comment branches, `content_hash` length, and `calls_per_function`'s populated content. Suite grows 6526 → 6549. - **`ast_parser`'s per-language extractors carried a dead return value, and two grammars had zero test coverage through their real call path** (boy-scout follow-on from #249; the resolver-dependent typecheck defect #249 itself named was independently closed by #253/#251's `_is_ast_language`/`SupportedLanguage`-derived-`AST_SUPPORTED` fix, already on `main`). Every per-language extractor (`_extract_python`/`_extract_js`/`_extract_go`/`_extract_swift`/`_extract_rust`, plus the JVM/C-family/scripting extractors `ast_extractor_registry.build_extra_extractors` composes) computed a flat `calls` list via `extract_calls_generic` and returned it as a 3rd tuple element that `parse_file_ast` immediately discarded — superseded by `calls_per_function`, the value actually consumed downstream. Removed; `Extractor` is now a 2-tuple `(imports, definitions)`. Swift and Rust parsing had never been exercised through `parse_file_ast` by any existing test (the Go equivalent bypassed its own wrapper by calling `extract_go_definitions` directly) — added `TestParseFileGo`/`TestParseFileSwift`/`TestParseFileRust`. Smaller gaps closed alongside: both branches of `is_available()`, the docstring/comment-extraction branches of `_node_text`/`_extract_module_doc` (via fake-`Node` unit tests), `content_hash`'s length invariant, `calls_per_function`'s populated content, and malformed-UTF-8 decode robustness (`errors="replace"`). Suite grows 6550 → 6573 (measured via `pytest --collect-only -q` against this repo's shared venv, matching CI's canonical collection per the parity this repo's `test_typecheck_env_parity.py`/`check_doc_claims.py` gates already establish). +- **`ast_parser`'s per-language extractors carried a dead return value, and two grammars had zero test coverage through their real call path** (boy-scout follow-on from #249; the resolver-dependent typecheck defect #249 itself named was independently closed by #253/#251's `_is_ast_language`/`SupportedLanguage`-derived-`AST_SUPPORTED` fix, already on `main`). Every per-language extractor (`_extract_python`/`_extract_js`/`_extract_go`/`_extract_swift`/`_extract_rust`, plus the JVM/C-family/scripting extractors `ast_extractor_registry.build_extra_extractors` composes) computed a flat `calls` list via `extract_calls_generic` and returned it as a 3rd tuple element that `parse_file_ast` immediately discarded — superseded by `calls_per_function`, the value actually consumed downstream. Removed; `Extractor` is now a 2-tuple `(imports, definitions)`. That element was the entire production call graph of `extract_calls_generic` (`mcp_server/core/ast_extractors.py`) — with it gone, the function had no caller left but its own direct unit test, so `extract_calls_generic` itself, its `TestCallExtraction` unit test, and the stale "also provides the generic call-site extractor used by all languages" line in the module docstring are removed/corrected too. Swift and Rust parsing had never been exercised through `parse_file_ast` by any existing test (the Go equivalent bypassed its own wrapper by calling `extract_go_definitions` directly) — added `TestParseFileGo`/`TestParseFileSwift`/`TestParseFileRust`. Smaller gaps closed alongside: both branches of `is_available()`, the docstring/comment-extraction branches of `_node_text`/`_extract_module_doc` (via fake-`Node` unit tests), `content_hash`'s length invariant, `calls_per_function`'s populated content, and malformed-UTF-8 decode robustness (`errors="replace"`). Suite grows 6550 → 6571 (measured via `pytest --collect-only -q` against this repo's shared venv, matching CI's canonical collection per the parity this repo's `test_typecheck_env_parity.py`/`check_doc_claims.py` gates already establish). - **A `created_at` that states a timezone was stored as the wrong instant** (#252). `normalize_date_to_iso` had no timezone policy on any of its paths, so three defects stacked: (1) the "already ISO" guard was the substring test `"T" in raw`, and every US zone abbreviation contains a T — `8 May 2023 13:56 EST` was returned unparsed; (2) the built-in fast path matches the date at the START of the string and discards the rest, so `8 May 2023 13:56 +02:00` became midnight, dropping both the time and the offset; (3) on the dateutil path an abbreviation it cannot resolve is dropped with a warning nobody sees, leaving a naive datetime that PostgreSQL's `timestamptz` cast and `compute_recency_boost` both read as UTC. The instant was up to a day off and nothing was emitted. A stated zone is now honoured or the value is refused: `mcp_server/core/temporal_timezones.py` supplies dateutil a `tzinfos` resolver over the **RFC 5322 §4.3 obs-zone table** (the normative answer to "which EST?" — cross-checked against CPython's `email._parseaddr._timezones`), and any abbreviation outside it is refused with a warning naming the input, the abbreviation and the fix, rather than defaulted. Parsing no longer depends on the host's local zone name, and no `warnings.catch_warnings()` — process-global and not thread-safe — is taken on a store write path. `normalize_date_to_iso` moves out of `core/temporal.py` into `core/temporal_normalize.py`: storage normalization must not lose precision, retrieval scoring may, and they are now separate modules (both stores import from the new path). The refusal also covers the degraded path — a string that states a zone is never salvaged to a naive date, whatever made the parse fail. - **`python-dateutil` is now a declared dependency** (#252). `normalize_date_to_iso` has always parsed free-form dates with a time of day through it — the LoCoMo shape `1:56 pm on 8 May, 2023` its own docstring cites — but it was never declared and arrived only by transitive luck. It was absent from every `requirements/ci-*.txt`, so **every CI test job ran with that parser missing**: the fallback was dead code in CI, and any install resolving without it kept dates it could not read. Declared, locked and hash-pinned into the 9 exported requirement sets, so the write path behaves identically on every install and both backends. - **Both stores skipped the normalization entirely for the dates that needed it most** (#252, the same substring defect one layer up). `PgMemoryStore._build_insert_params` and `SqliteMemoryStore._insert_memory_rows` each guarded the call with `"T" not in raw_created` as a cheap "is it already ISO?" test — so `8 May 2023 13:56 EST` (and every `PST`/`CST`/`MST` string) went to the database untouched, whichever way `normalize_date_to_iso` behaved. The guard is gone from both: deciding what is already ISO belongs to the function that owns it, which returns a real ISO datetime unchanged. Asserted on the stored row, not just on the parser, and asserted equal across the two backends. diff --git a/mcp_server/core/ast_extractor_registry.py b/mcp_server/core/ast_extractor_registry.py index f20a420a..b626f345 100644 --- a/mcp_server/core/ast_extractor_registry.py +++ b/mcp_server/core/ast_extractor_registry.py @@ -48,11 +48,14 @@ def _make_extractor( Precondition: `imports_fn`/`defs_fn` are pure (no I/O), taking the same `(root, source)` pair. Postcondition: returns a callable producing `(imports, definitions)` — - the flat per-file call list this used to also return - (`extract_calls_generic(root, source)`) was computed and then discarded - by every caller (`parse_file_ast` only reads the `calls_per_function` - map, populated separately via `extract_calls_per_function`); removed as - dead code (issue #249 boy-scout pass) rather than given a test. + the flat per-file call list this used to also return was computed via + `extract_calls_generic(root, source)` and then discarded by every + caller (`parse_file_ast` only reads the `calls_per_function` map, + populated separately via `extract_calls_per_function`); the tuple + element was removed as dead code (issue #249 boy-scout pass). With + this the only production call site gone, `extract_calls_generic` had + no caller left but its own direct unit test — deleted from + `ast_extractors.py` rather than kept for a hypothetical future one. """ def _extract(root: Node, source: bytes) -> tuple[list[ImportInfo], list[SymbolDef]]: diff --git a/mcp_server/core/ast_extractors.py b/mcp_server/core/ast_extractors.py index 558a8c76..6650532a 100644 --- a/mcp_server/core/ast_extractors.py +++ b/mcp_server/core/ast_extractors.py @@ -1,7 +1,6 @@ """Tree-sitter AST extractors for Python and JavaScript/TypeScript. Additional languages (Go, Swift, Rust) in ast_extractors_extra.py. -Also provides the generic call-site extractor used by all languages. Pure functions — no I/O. """ @@ -217,25 +216,6 @@ def _extract_js_class( _extract_js_node(child, source, defs, cls_name) -# ── Generic call extraction ────────────────────────────────────────────── - - -def extract_calls_generic(root: Node, source: bytes) -> list[str]: - """Extract all function/method call names from AST.""" - calls: list[str] = [] - seen: set[str] = set() - for call_type in ("call", "call_expression"): - for node in _walk_type(root, call_type): - func = node.child_by_field_name("function") - if not func: - continue - name = _text(func, source).strip() - if name and name not in seen and len(name) < _MAX_CALL_NAME_LEN: - calls.append(name) - seen.add(name) - return calls - - # ── Per-function call extraction (caller-qualified) ───────────────────── # Tree-sitter node types that represent a function/method definition diff --git a/tests_py/core/test_ast_extractors.py b/tests_py/core/test_ast_extractors.py index 75c5f4cb..1158a57a 100644 --- a/tests_py/core/test_ast_extractors.py +++ b/tests_py/core/test_ast_extractors.py @@ -119,42 +119,6 @@ def test_interface_and_type(self) -> None: assert r.language == "typescript" -class TestCallExtraction: - def test_python_calls(self) -> None: - code = b""" -result = process(data) -x = helper(1, 2) -obj.method() -""" - if _HAS_TREE_SITTER: - from mcp_server.core.ast_extractors import extract_calls_generic - from tree_sitter_language_pack import get_parser - - tree = get_parser("python").parse(code) - calls = extract_calls_generic(tree.root_node, code) - assert "process" in calls - assert "helper" in calls - else: - r = parse_file_ast("test.py", code) - assert r.language == "python" - - def test_js_calls(self) -> None: - code = b""" -const x = fetchData(url); -console.log("hello"); -""" - if _HAS_TREE_SITTER: - from mcp_server.core.ast_extractors import extract_calls_generic - from tree_sitter_language_pack import get_parser - - tree = get_parser("javascript").parse(code) - calls = extract_calls_generic(tree.root_node, code) - assert any("fetchData" in c for c in calls) - else: - r = parse_file_ast("app.js", code) - assert r.language == "javascript" - - class TestGoExtractors: def test_go_struct_and_method(self) -> None: code = b""" From 6df77ac8acfb7e42f52ed38c8c37b34dc703fe2b Mon Sep 17 00:00:00 2001 From: cdeust Date: Thu, 30 Jul 2026 03:44:56 +0200 Subject: [PATCH 4/6] =?UTF-8?q?refactor(ast):=20split=20extract=5Fcalls=5F?= =?UTF-8?q?per=5Ffunction's=20nested=20walk=20under=20the=2040-line/method?= =?UTF-8?q?=20cap=20(boy-scout,=20CLAUDE.md=20=C2=A74)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seen while touching this file for the extract_calls_generic deletion: extract_calls_per_function (51 lines including its nested `walk` closure) already violated this repo's 40-line/method cap on main, pre-existing and unrelated to either fix in this branch. Fixed in place since it is squarely in this diff's blast radius (coding-standards.md §14.1) and the split carries zero external risk — the public `extract_calls_per_function(root, source) -> dict[str, list[str]]` signature and behavior are unchanged, only the internals move. Split into three functions, each under the cap: `extract_calls_per_function` (17 lines, unchanged behavior/contract) delegates to `_walk_for_calls` (36 lines, the former nested closure — now a named top-level helper taking `out` as an explicit accumulator parameter instead of a captured free variable) which delegates per-function call collection to `_collect_call_basenames` (10 lines, the former inner per-call-type loop). Pure extract-method refactor: no branch, condition, or dedup logic changed. Not addressed here: this file's own line count (330 after this commit, still over the repo's 300-line/file cap — it was already 337 before either commit in this branch touched it). A real fix needs a genuine module split (this file has 6 importers: ast_parser.py plus the clike/extra/jvm/scripting extractor siblings and its own test file — Move-7's >5-importers High-stakes trigger), which means updating import statements in 5 other files. That is a separate, dedicated behavior-preserving-refactor PR per coding-standards.md §15.1 ("never ship the connection half-made... never mix the enabling refactor and the feature in one PR"), not a rider on a rebase-plus-one-review-finding PR already carrying real risk from two prior CONFLICTING states. Flagged in this session's report for the coordinator rather than filed as an issue (explicit instruction: no new issues this round). Verification: tests_py/core/test_ast_parser.py + test_ast_extractors_multilang.py + test_ast_extractors.py + test_ast_parser_language_contract.py (69 passed, unchanged), tests_py/core/ (3384 passed, unchanged), ruff check + ruff format --check clean, pyright 0 errors/0 warnings on the changed file. Full suite count unchanged at 6571 (no tests added or removed by this commit). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u --- mcp_server/core/ast_extractors.py | 87 ++++++++++++++++++------------- 1 file changed, 50 insertions(+), 37 deletions(-) diff --git a/mcp_server/core/ast_extractors.py b/mcp_server/core/ast_extractors.py index 6650532a..58c17e45 100644 --- a/mcp_server/core/ast_extractors.py +++ b/mcp_server/core/ast_extractors.py @@ -277,41 +277,54 @@ def extract_calls_per_function( qname means we can't attach edges to them. """ out: dict[str, list[str]] = {} - - def walk(node: Node, class_scope: str) -> None: - for child in node.children: - ntype = child.type - if ntype in _CLASS_NODE_TYPES: - name_node = child.child_by_field_name("name") - cls = _text(name_node, source) if name_node else class_scope - body = child.child_by_field_name("body") or child - walk(body, cls or class_scope) - elif ntype == "decorated_definition": - walk(child, class_scope) - elif ntype in _FUNCTION_NODE_TYPES: - name_node = child.child_by_field_name("name") - fn_name = _text(name_node, source) if name_node else "" - body = child.child_by_field_name("body") or child - if fn_name: - qname = f"{class_scope}.{fn_name}" if class_scope else fn_name - calls: list[str] = [] - seen: set[str] = set() - for call_type in _CALL_NODE_TYPES: - for call in _walk_type(body, call_type): - base = _callee_basename(call, source) - if ( - base - and base not in seen - and len(base) < _MAX_CALL_NAME_LEN - ): - calls.append(base) - seen.add(base) - out[qname] = calls - # Recurse into body for nested definitions (inner - # functions, closures that define named functions). - walk(body, class_scope) - else: - walk(child, class_scope) - - walk(root, "") + _walk_for_calls(root, "", source, out) return out + + +def _walk_for_calls( + node: Node, + class_scope: str, + source: bytes, + out: dict[str, list[str]], +) -> None: + """Recurse through ``node``'s children, tracking the enclosing class. + + Precondition: `out` is the accumulator `extract_calls_per_function` + returns; this function only adds keys, it does not read existing ones. + Postcondition: every named function/method reachable from `node` has + its qualified name mapped to its deduped callee-basename list in `out`. + """ + for child in node.children: + ntype = child.type + if ntype in _CLASS_NODE_TYPES: + name_node = child.child_by_field_name("name") + cls = _text(name_node, source) if name_node else class_scope + body = child.child_by_field_name("body") or child + _walk_for_calls(body, cls or class_scope, source, out) + elif ntype == "decorated_definition": + _walk_for_calls(child, class_scope, source, out) + elif ntype in _FUNCTION_NODE_TYPES: + name_node = child.child_by_field_name("name") + fn_name = _text(name_node, source) if name_node else "" + body = child.child_by_field_name("body") or child + if fn_name: + qname = f"{class_scope}.{fn_name}" if class_scope else fn_name + out[qname] = _collect_call_basenames(body, source) + # Recurse into body for nested definitions (inner + # functions, closures that define named functions). + _walk_for_calls(body, class_scope, source, out) + else: + _walk_for_calls(child, class_scope, source, out) + + +def _collect_call_basenames(body: Node, source: bytes) -> list[str]: + """Deduped, order-preserving callee basenames for every call under `body`.""" + calls: list[str] = [] + seen: set[str] = set() + for call_type in _CALL_NODE_TYPES: + for call in _walk_type(body, call_type): + base = _callee_basename(call, source) + if base and base not in seen and len(base) < _MAX_CALL_NAME_LEN: + calls.append(base) + seen.add(base) + return calls From 2eac4c33fdd49d5f98550b4ef7f66573bf1b6eb5 Mon Sep 17 00:00:00 2001 From: cdeust Date: Thu, 30 Jul 2026 15:13:52 +0200 Subject: [PATCH 5/6] chore: rebase onto main and resync the advertised suite size to 6777 Rebase bookkeeping only. Conflicts were confined to the six files that hard-code the suite size, and only hunks differing solely by that number were resolved (main's figure wins); any hunk with real content divergence aborts the rebase for human/agent resolution instead of being guessed at. Count is the measured absolute from a real collection. The branch's own touched tests were re-run after resolution to catch content loss. --- assets/badge-tests.svg | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/assets/badge-tests.svg b/assets/badge-tests.svg index 8e770857..66cf847e 100644 --- a/assets/badge-tests.svg +++ b/assets/badge-tests.svg @@ -1,5 +1,5 @@ - - 6768 tests passing + + 6777 tests passing