Orchestrata Ultimate › Plans › bodymatcher-split-plan

BodyMatcher split plan (cohesion-driven, behavior-preserving)

Source: plans/bodymatcher-split-plan.md at 8a2cb42ad · last changed 2026-07-27 · first committed 2026-07-09 · 205 lines.

Prose pending.

Plan text

BodyMatcher split plan (cohesion-driven, behavior-preserving)

Status: EXECUTED 2026-07-10 (commits 5c7b74f6.. on master), with two mid-flight user rulings that superseded parts of this plan:

1. "Why not AST?" → InheritanceMap sources the serializer's own inheritance_graph (resolved bases) instead of re-walking ClassInfo.bases; the walk survives only as the fallback for synthetic codebases. Byte-identical on all gates.
2. "Separate the AST generation into its own component" + "no more writing inheritance checks, just ast — enhance that if it needs it" → the generation core (models, CodebaseAnalyzer, serialize) was extracted from scripts/serialize_codebase_ast.py into the new stdlib-only src/orchestrata_ast/, and InheritanceMap moved INTO that component (orchestrata_ast.inheritance) instead of living in orchestrata_tools as §2 below planned. The script re-exports every moved name; serialization output is byte-identical.

Outcome vs targets: body_matcher.py 542→360 lines; worst function complexity 35→23 (_parse_file, target ≤15 NOT reached — the remaining bulk is file-discovery/orchestration; acceptable residual). All equivalence gates (find_matches rows, direct index, writer dry-run, serialize JSON) byte-identical at every step. Follow-up debt recorded: scripts/serialize_codebase_ast.py carries 195 pre-existing ERROR-level mypy findings (was 282 before extraction) — separate cleanup.

The original plan below is retained for the record.

Why (measured, not vibes)

src/orchestrata_tools/test_analyzer/body_matcher.py — 542 lines, one class, 18 methods, four distinct responsibilities. Findings that motivated this:

The (calls, used_imports, var_types) anonymous 3-tuple is threaded through five signatures and unpacked as fc, fi, ft three times in _index_test alone — that shape IS the hidden seam between the responsibilities.

Current responsibilities (the cohesion map)

1. Per-file AST extraction (pure; no object state): _parse_file's walk, _extract_calls_and_types, _get_call_name, _get_decorator_name, _get_annotation_name, _is_pytest_fixture, _get_fixture_params.
2. Fixture registry + resolution: _module_fixtures, class-fixture collection in _parse_file, _find_conftest_fixture, and the merge loop in _index_test.
3. Inheritance knowledge: _build_inheritance_map, _is_subclass, is_subclass_of, plus the test-file base-class collection in _parse_file.
4. Index + query facade: _build_index (file discovery), _calls, _test_imports, _direct_calls, find_tests_for_method, is_tested, direct_call_index.

Target layout (3 files, 1 shared dataclass)

1. test_analyzer/call_extraction.py (~200 lines, all pure)

@dataclass(frozen=True)
class CallProfile:
    """What one function's body does: resolved call names, the imported
    classes it uses, and inferred variable types."""
    calls: frozenset[str]
    used_imports: frozenset[str]
    var_types: Mapping[str, str]   # var name -> class name

    def merged_with(self, other: "CallProfile") -> "CallProfile": ...

class CallExtractor:
    """Stateless per-node extraction. Everything currently in
    _extract_calls_and_types + the _get_* name helpers + fixture-param
    detection."""
    def extract(self, node, imported_names, imported_classes,
                local_classes) -> CallProfile: ...
    def fixture_params(self, node) -> list[str]: ...
    def is_pytest_fixture(self, node) -> bool: ...
    # module-level helpers: get_call_name(), get_decorator_name(),
    # get_annotation_name()

CallProfile replaces every tuple[set[str], set[str], dict[str, str]] annotation and the fc, fi, ft unpacking. _index_test drops from 9 parameters to 4 (node, mod_path, class_name, profile-sources).

2. test_analyzer/inheritance.py (~60 lines)

class InheritanceMap:
    """class name -> ancestor names, built from a source ASTCodebase and
    augmented with class defs found in test files."""
    @classmethod
    def from_codebase(cls, source_ast: ASTCodebase | None) -> "InheritanceMap": ...
    def add(self, child: str, bases: Iterable[str]) -> None: ...
    def is_subclass(self, child: str, parent: str) -> bool: ...   # child==parent counts
    def ancestors(self, cls_name: str) -> frozenset[str]: ...

Kills two duplications at once:

• TestMatcher.is_imported's reach-in to _body_matcher._is_subclass becomes a call on the shared map.
• scripts/add_traces_from_evidence.py::_ancestor_methods swaps its hand-rolled recursive base walk for ancestors() + a class lookup.

3. test_analyzer/body_matcher.py (facade, ~250 lines)

Keeps the name, the public API, and the indexes:

• __init__(test_ast, source_ast=None) — signature unchanged.
• _build_index / _parse_file stay here but shrink to orchestration: discover files → collect imports/local classes → CallExtractor.extract per function → FixtureRegistry.register → _index_test (now: subtract decorator names, store direct profile, merge fixture profiles, fill _calls/_test_imports).
• FixtureRegistry (private class, ~50 lines, lives in this file): wraps _module_fixtures + _find_conftest_fixture + the three-way class/module/conftest lookup, returning CallProfile | None for a param. Not worth its own file until a second consumer exists.
• Public, unchanged: find_tests_for_method, is_tested, direct_call_index, is_subclass_of (delegates to InheritanceMap).
• New public (to delete the matcher.py reach-in): class_used_by_test(test_module, class_name) -> bool — encapsulates the _test_imports scan + subclass acceptance that TestMatcher.is_imported currently does by hand.

Consumer diffs (complete list — verified by grep, no other importers)

What does NOT change

• Class name BodyMatcher, module path, constructor signature.
• Every public method signature and return shape.
• Matching semantics: same patterns 1/1b/2/3 in find_tests_for_method, same fixture-resolution order (class → module → conftest nearest-first), same decorator subtraction in the direct index.
• No new dependencies; no changes outside src/orchestrata_tools + the one helper swap in scripts/add_traces_from_evidence.py.

Equivalence gates (before AND after, must be identical)

1. Match-set byte equality: dump TestMatcher(test_ast).find_matches(source_index) on the real snapshots (plans/ast-tests.json × plans/ast-ultimate.json) to a sorted JSON list of (test_module, test_name, method_fqn, strategy, confidence); diff pre vs post must be empty.
2. Writer dry-run equality: python scripts/add_traces_from_evidence.py stdout must be byte-identical pre vs post (currently 0 new, 0 merges, 43 already-present, across 27 files).
3. Direct-index equality: serialize direct_call_index() sorted; diff empty.
4. Test suites green both sides: tests/test_orchestrata_tools.py (55), tests/test_orchestrata_tools_body_matcher.py (6), tests/scripts/test_add_traces_from_evidence.py (12), tests/scripts/test_generate_rtm_evidence.py (12), plus the full-suite run before commit.
5. Static gates: ruff, black, pyright ratchet ≤283, untyped ≤0, code-review ERRORs = 0 on the touched files; complexity re-measured — target: no function ≥ 15 (from 35/18/17 today).

New tests owed by the split

• CallProfile.merged_with (union semantics, var_types precedence = incoming overrides — matches today's dict.update order).
• InheritanceMap: transitive ancestors, cycle tolerance (A→B→A must not recurse forever — today's _build_inheritance_map guards via membership check; keep a regression test), test-file augmentation.
• class_used_by_test: direct import, subclass-of-imported, absent.

Execution order (single PR, ~6 commits)

1. inheritance.py + tests; BodyMatcher delegates; matcher.py reach-in for _is_subclass replaced. Gates.
2. CallProfile + call_extraction.py + tests; BodyMatcher delegates extraction; tuple threading replaced. Gates.
3. FixtureRegistry private class; _index_test slims to ≤5 params. Gates.
4. class_used_by_test public API + matcher.py _test_imports reach-in removed + tests. Gates.
5. add_traces_from_evidence._ancestor_methods → InheritanceMap. Gates incl. writer dry-run equality.
6. Full suite + equivalence dumps + complexity re-measure; commit series.

Risks

• _parse_file complexity-35 untangling is the only genuinely risky step; mitigation is gate #1 (match-set byte equality) which covers every semantic path the parser feeds.
• Frozen CallProfile vs today's in-place |= mutation in the fixture merge loop: merged_with allocates per fixture param. Cost is negligible (hundreds of tests × ≤4 fixtures), and immutability is what stops the aliasing bug class where a fixture's sets get mutated by a later test.
• Formatter hook strips imports that are momentarily unused — apply each commit's import + usage in the same edit (known failure mode, hit twice this week).

Related

• Plans
• Orchestrata Ultimate — project index
• Workflow
• Tasks