Skip to content

chore(ast): remove dead call-tuple element + delete extract_calls_generic + close Swift/Rust test gaps (follow-on to #249, closed by #261) - #270

Open
cdeust wants to merge 4 commits into
mainfrom
fix-typecheck-resolver-dependent-249
Open

chore(ast): remove dead call-tuple element + delete extract_calls_generic + close Swift/Rust test gaps (follow-on to #249, closed by #261)#270
cdeust wants to merge 4 commits into
mainfrom
fix-typecheck-resolver-dependent-249

Conversation

@cdeust

@cdeust cdeust commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Follow-on to #249 — that issue's own resolver-dependent typecheck defect was independently closed by #253/#251 (merged as #261, now on main) via the same mechanism this PR originally proposed (a TypeGuard narrowing language: str to the language pack's own SupportedLanguage Literal, with AST_SUPPORTED derived from _EXTRACTORS rather than restated). Rebasing this branch onto current main surfaced that overlap directly in ast_parser.py; this PR now keeps main's mechanism and drops this branch's own duplicate _SupportedLanguage/_is_ast_supported/get_args(...) construction, which would otherwise have shipped a second, divergent narrowing beside the one already merged.

What this PR still does (the part #261 didn't)

Boy-scout pass (§14) on mcp_server/core/ast_parser.py / ast_extractor_registry.py, from the original #249 investigation:

  • Dead code removed. 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. Extractor is now a 2-tuple (imports, definitions).
  • extract_calls_generic itself deleted (review finding). Stripping that 3rd tuple element left extract_calls_generic (mcp_server/core/ast_extractors.py) with zero production callers — only its own two-test TestCallExtraction class called it directly. Deleted the function and that test class, corrected the module docstring's now-false "Also provides the generic call-site extractor used by all languages" claim, and reworded ast_extractor_registry._make_extractor's docstring to record that the function itself, not just its tuple slot, is gone. _MAX_CALL_NAME_LEN/_walk_type stay live — both still back extract_calls_per_function, the caller-qualified version parse_file_ast actually consumes.
  • extract_calls_per_function's 40-line/method cap violation (boy-scout, seen while in this file for the deletion above). Pre-existing on main (51 lines including a nested walk closure), unrelated to either fix in this branch, but squarely in this diff's blast radius once the file was touched. Split into extract_calls_per_function (17 lines, contract unchanged) → _walk_for_calls (36 lines, the former closure, now a named helper taking the accumulator as an explicit parameter) → _collect_call_basenames (10 lines, the former inner per-call-type loop). Pure extract-method refactor, zero behavior change, verified by the unchanged test suite passing unmodified.
  • Not addressed (reported, not filed): this file is still 330 lines, over the repo's 300-line/file cap (it was already 337 before either commit in this branch touched it — this PR only ever reduces it). A real fix needs a genuine module split; the file has 6 importers (ast_parser.py + the clike/extra/jvm/scripting extractor siblings + its own test file — Move-7's >5-importers High-stakes trigger), so splitting it means touching import statements in 5 other files. That's a separate, dedicated behavior-preserving-refactor PR per coding-standards.md §15.1, not a rider on a rebase-plus-review-finding PR that already carried real risk from two prior CONFLICTING states. Left for the coordinator to schedule rather than filed as an issue per this round's explicit no-new-issues instruction.
  • Zero test coverage closed. _extract_swift/_extract_rust were never 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: both branches of is_available(), _node_text/_extract_module_doc's docstring/comment-extraction branches (fake-Node unit tests), content_hash's length invariant, calls_per_function's populated content, and malformed-UTF-8 decode robustness (errors="replace").

Filed #269 for 50 mutmut "survived" reports left after the original mutation pass — independently verified as false positives (looping MUTANT_UNDER_TEST over every one against the correct, full test selection: every single one fails). Root cause: mutmut's coverage-based test-selection optimizer undercounts callers of a dispatch table (_EXTRACTORS) built once, eagerly, at module-import time — a tooling limitation orthogonal to this issue.

Rebase reconciliation (rebased onto main twice — after #261/#272, then again after #264/#268/#271/#274)

  • ast_parser.py / ast_extractor_registry.py: resolved on the merits — kept main's _is_ast_language/pack-SupportedLanguage/derived-AST_SUPPORTED mechanism, kept this branch's 2-tuple dead-code removal on top of it (the two are independent and compose cleanly; ast_extractor_registry.py auto-merged with zero conflicts both times).
  • .github/workflows/ci.yml: kept main's "Record the resolved type surface" step (prints pyright + tree-sitter + tree-sitter-language-pack versions), which is a superset of this branch's own single-package version-print step; dropped the latter as redundant.
  • Suite-size files (CLAUDE.md, README.md, CONTRIBUTING.md, .bestpractices.json, docs/ASSURANCE-CASE.md, assets/badge-tests.svg): recomputed twice from a live pytest --collect-only -q on this machine each time — never a delta from either side's advertised number (this machine's collection is proven to match CI's canonical exactly, per fix(core): honour or refuse a stated timezone in normalize_date_to_iso (#252) #259). First rebase landed at 6573 (main's 6550 + this branch's +23); deleting extract_calls_generic's own two-test class dropped it to the final 6571. scripts/check_doc_claims.py --test-count 6571 and scripts/generate_repo_badges.py --check --test-count 6571 both green, badge regenerated by the actual generator script and diffed byte-identical against the manual conflict resolution.
  • CHANGELOG's #249 entry rewritten: no longer claims to close typecheck gate is resolver-dependent: pyright reports 1 error under uv.lock, 0 under CI's pip resolution (ast_parser.py:88) #249 (already closed by fix(deps): pin tree-sitter-language-pack to the range where get_parser().parse(bytes) works (#253) #261); describes only the boy-scout dead-code-removal (including the extract_calls_generic deletion) and test-coverage work this branch still contributes. A stale "6549 → 6572" prose claim inside the entry itself (not part of any rebase conflict — git doesn't touch unconflicted prose) was caught by grepping the file content post-rebase, not just checking git status, and corrected to "6550 → 6571".

Completion Ledger

Path introduced by this diff Asserting test
_extract_python/_extract_js/_extract_go/_extract_swift/_extract_rust 2-tuple return TestParseFilePython, TestParseFileTypeScript, TestParseFileGo, TestParseFileSwift, TestParseFileRust (imports + definitions assertions)
ast_extractor_registry._make_extractor/build_extra_extractors 2-tuple return TestParseFileTypeScript + test_ast_extractors_multilang.py (java/kotlin/c/cpp/csharp/ruby/php)
extract_calls_generic deletion — no remaining reference in mcp_server/ or tests_py/ grep -rn extract_calls_generic . (repo-wide, post-deletion) returns only the explanatory prose in ast_extractor_registry.py's docstring and this CHANGELOG entry, no code reference
extract_calls_per_function_walk_for_calls + _collect_call_basenames split (behavior-preserving) test_calls_per_function_populated + the full TestParseFile* family (all pass unmodified — same assertions, same call graph output)
is_available() True branch test_is_available_reflects_installed_tree_sitter
is_available() False branch (ImportError) test_is_available_false_when_tree_sitter_import_fails
_node_text/_extract_module_doc branches (7 distinct paths) TestNodeText, TestExtractModuleDoc (7 focused fake-Node tests)
content_hash length invariant test_content_hash_length
calls_per_function populated content test_calls_per_function_populated
malformed-UTF-8 decode robustness (errors="replace") test_decodes_malformed_utf8_without_raising, TestNodeText::test_replaces_invalid_utf8_instead_of_raising
main's _is_ast_language/SupportedLanguage-derived AST_SUPPORTED (unchanged by this PR, verified still intact post-rebase) test_ast_parser_language_contract.py (all cases), test_ast_extractors_multilang.py::test_all_langs_registered
Recomputed suite count (6571) in doc-claim files scripts/check_doc_claims.py --test-count 6571, scripts/generate_repo_badges.py --check --test-count 6571 (both run manually; also enforced live in CI's Lint/doc-claims jobs)

Test plan

  • pytest tests_py/core/test_ast_parser.py tests_py/core/test_ast_extractors_multilang.py tests_py/core/test_ast_extractors.py tests_py/core/test_ast_parser_language_contract.py: 69 passed
  • pytest tests_py/core/: 3384 passed (no regression)
  • pytest (full suite): 6571 passed, 117 subtests passed — exact match to pytest --collect-only -q, run twice (before and after the extract_calls_per_function split)
  • ruff check + ruff format --check on all changed .py files: clean
  • pyright on changed .py files: 0 errors, 0 warnings
  • scripts/check_doc_claims.py --test-count 6571 / scripts/generate_repo_badges.py --check --test-count 6571: both green
  • tests_py/scripts/test_typecheck_env_parity.py, test_check_doc_claims.py, test_generate_repo_badges.py: 77 passed, 30 subtests passed
  • CI on pushed head b8fb187: all 19 checks green (gh pr checks 270)

🤖 Generated with Claude Code

https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u

@cdeust
cdeust force-pushed the fix-typecheck-resolver-dependent-249 branch 3 times, most recently from ddec2f4 to 8a5ed4d Compare July 29, 2026 22:45
@cdeust cdeust changed the title fix(types): narrow ast_parser's language str to a Literal, closing the resolver-dependent typecheck gate chore(ast): remove dead call-tuple element + close Swift/Rust test gaps in ast_parser (follow-on to #249, closed by #261) Jul 29, 2026
cdeust and others added 3 commits July 30, 2026 03:00
…e 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u
… CHANGELOG claim

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u
…as the last caller (#249 review)

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u
@cdeust
cdeust force-pushed the fix-typecheck-resolver-dependent-249 branch from 8a5ed4d to 4727630 Compare July 30, 2026 01:20
@cdeust cdeust changed the title chore(ast): remove dead call-tuple element + close Swift/Rust test gaps in ast_parser (follow-on to #249, closed by #261) chore(ast): remove dead call-tuple element + delete extract_calls_generic + close Swift/Rust test gaps (follow-on to #249, closed by #261) Jul 30, 2026
…he 40-line/method cap (boy-scout, CLAUDE.md §4)

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

typecheck gate is resolver-dependent: pyright reports 1 error under uv.lock, 0 under CI's pip resolution (ast_parser.py:88)

1 participant