Skip to content

fix(parser): disambiguate Python and Ruby methods in same-leaf classes - #1087

Open
azizur100389 wants to merge 4 commits into
tirth8205:stagingfrom
azizur100389:codex/fix-nested-method-identities
Open

azizur100389 wants to merge 4 commits into
tirth8205:stagingfrom
azizur100389:codex/fix-nested-method-identities

Conversation

@azizur100389

@azizur100389 azizur100389 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Linked issue

Closes #999.

What & why

Python and Ruby methods in Alpha.Same and Beta.Same share identities and overwrite each other. Index ambiguous class leaf scopes once per file, carry the full lexical scope into affected method identities and CONTAINS/call sources, and prefer same-scope calls rather than another class's method. Keep unique-leaf method identities unchanged and leave a missing local member unbound. Rebuild existing graphs to replace overwritten rows.

How it was tested

Applied the same regression/affected-module/full-baseline methodology used in my previous PRs (#274, #276, #279, #353 and #354), against unchanged staging 6b12d11625cbec3b6773e076cb3d136464fa90e5.

  • Final regression module: 6 passed. Bugs reproduce on unchanged staging; compatibility guards cover existing behavior.
  • Affected modules: 671 passed, 13 existing Windows failures. Failure identities are a subset of the unchanged-staging affected baseline (1,169 passed, 15 Windows failures); no new failures.
  • Ruff clean; Bandit zero findings; git diff --check clean.
  • Mypy: the same three pre-existing Windows fcntl LOCK_EX/LOCK_NB/flock errors in daemon.py as staging; no new errors.
  • Schema remains version 13.
python -m pytest tests/test_nested_method_identities.py tests/test_parser.py tests/test_multilang.py tests/test_python_receiver_evidence.py tests/test_python_receiver_shapes.py -q --tb=short
$env:GIT_CEILING_DIRECTORIES = $env:TEMP.Replace('\\', '/') + ';' + (Split-Path $PWD).Replace('\\', '/')
python -m pytest -q --tb=short -m 'not browser and not upgrade' --cov=code_review_graph --cov-report=term --cov-fail-under=65
ruff check code_review_graph tests/test_nested_method_identities.py
mypy code_review_graph --ignore-missing-imports --no-strict-optional
bandit -r code_review_graph -c pyproject.toml

GitHub CI: 17 applicable checks passed on the final commit, including Python 3.10–3.13, browser tests, native Windows tests, and the three MCP-client platforms. Python 3.13: 4,274 passed, 804 skipped, 46 deselected, 2 xfailed, 2 xpassed; 85.88% coverage (65% required). The manual upgrade job is skipped by design.

Full local Windows suite: 4,147 passed, 112 failed, 819 skipped, 46 deselected, 2 xfailed, 2 xpassed; 86% coverage. Unchanged staging full baseline: 4,143 passed, 110 failed. The two additional failures are the callers_of default token-budget and affected-flows worst-case token-budget tests. Both also fail on unchanged staging with comparable long workspace temporary paths (supplemental baseline: 2/2 failed). They reflect absolute-path length in response sizes; no production budget or assertion was weakened. PATH/PYTHONPATH select the isolated checkout, and Git ceilings keep fixture repositories away from the enclosing home Git repository. Browser tests passed in CI; browser and opt-in upgrade tests were excluded locally.

All ten issue fixes were additionally tested together: 283 regression checks passed, and the combined affected suite had 1,142 passed / 25 unchanged-staging Windows failures, with no new failure identities. The tested combination is preserved in this integration branch.

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

code-review-graph review

Overall risk: 0.65 (MEDIUM) — 31 changed function(s)/class(es), 46 affected flow(s), 3 test gap(s) (1 with no tested caller found, 2 reached only through a caller)

Risk-scored changes

Risk Level Symbol Location Tested
0.65 medium code_review_graph/parser.py::CodeParser._index_ambiguous_method_scopes code_review_graph/parser.py:11845 yes
0.55 medium code_review_graph/parser.py::CodeParser code_review_graph/parser.py:2942 yes
0.40 medium code_review_graph/parser.py::CodeParser._resolve_call_targets code_review_graph/parser.py:5259 indirect
0.35 low code_review_graph/parser.py::CodeParser._extract_bytes code_review_graph/parser.py:3191 indirect
0.35 low code_review_graph/parser.py::CodeParser._extract_functions code_review_graph/parser.py:11871 no
0.30 low tests/test_nested_method_identities.py::_write_repo tests/test_nested_method_identities.py:502 (test)
0.30 low tests/test_nested_method_identities.py::_graph_snapshot tests/test_nested_method_identities.py:510 (test)
0.25 low tests/test_nested_method_identities.py::_contains_sources tests/test_nested_method_identities.py:113 (test)
0.15 low tests/test_nested_method_identities.py::_parse tests/test_nested_method_identities.py:97 (test)
0.15 low tests/test_nested_method_identities.py::_methods tests/test_nested_method_identities.py:105 (test)

Affected execution flows

  • process — criticality 0.74, 539 node(s) across 14 file(s)
  • _run — criticality 0.74, 566 node(s) across 16 file(s)
  • build_or_update_graph_tool — criticality 0.73, 620 node(s) across 24 file(s)
  • main — criticality 0.72, 926 node(s) across 40 file(s)
  • run_token_benchmark — criticality 0.70, 38 node(s) across 5 file(s)
  • ...and 41 more affected flow(s)

Test gaps

  • code_review_graph/parser.py::CodeParser._extract_functions (code_review_graph/parser.py:11871)

Reached only through a caller

No test names these directly, but a tested caller reaches them along the call path shown. That is a path in the call graph, not a record of execution -- the caller's tests may never take this branch. Treat it as where to look, not as coverage: these count as gaps and are scored as untested.

  • code_review_graph/parser.py::CodeParser._extract_bytes (code_review_graph/parser.py:3191) — via code_review_graph/parser.py::CodeParser.parse_bytes, 1 hop(s)
  • code_review_graph/parser.py::CodeParser._resolve_call_targets (code_review_graph/parser.py:5259) — via code_review_graph/parser.py::CodeParser.parse_bytes, 2 hop(s)

Powered by code-review-graph — local-first analysis; no code leaves the CI runner.

@azizur100389
azizur100389 marked this pull request as ready for review October 1, 2026 18:00
@tirth8205

Copy link
Copy Markdown
Owner

Thank you for this PR. I tested it with same-name classes in Python and Ruby files.

Before I can merge it, please fix these:

  1. A top-level Ruby class can call a method of the wrong class. This happens when a nested class with the same name comes first in the file. The cause is in code_review_graph/parser.py:5305. disambiguated_scopes only keeps names that contain a dot, so the top-level User is left out. Example: module Admin; class User; def save; validate(); end; def validate; end; end; end and then class User; def save; validate(); self.validate; end; def validate; end; end. Both calls in the second User#save go to Admin.User.validate, not to User.validate. The failing test is test_top_level_ruby_class_calls_stay_in_that_class[nested-first] in tests/test_nested_method_identities.py.

The failing tests are on the branch review/pr-1087 in this repo. You can copy them from there. If a test there is marked xfail, please remove that mark with your fix.

After you push the fix, I will test it again.

@azizur100389

Copy link
Copy Markdown
Contributor Author

Hi @tirth8205, the review issues have been fixed and pushed, with regression tests covering the reported cases.

Top-level Ruby classes sharing a name with nested classes now resolve implicit and self calls within the correct class, including nested-first declaration order. Your regression test is included with its xfail mark removed.

I rechecked CI today: all 17 checks are passing, with the upgrade-path check intentionally skipped. The branch is also up to date with staging and has no merge conflicts.

Could you please retest the updated PR when you have a chance? Thank you!

This branch has not been deployed

No deployments
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.

[Bug]: method identities are namespace-shallow: two same-leaf classes in one file share one Function qualified_name

2 participants