Skip to content

revert: undo PR #455 due to 30 test regressions across multiple languages#467

Merged
vitali87 merged 1 commit intomainfrom
revert/pr-455-call-resolver
Mar 21, 2026
Merged

revert: undo PR #455 due to 30 test regressions across multiple languages#467
vitali87 merged 1 commit intomainfrom
revert/pr-455-call-resolver

Conversation

@vitali87
Copy link
Owner

Summary

Reverts PR #455 (JS/TS service call resolution and empty-graph reindex) which introduced 30 test failures across Python, Lua, Rust, C++, TypeScript, and JavaScript.

Root cause

PR #455 modified call_resolver.py and js_ts/type_inference.py — core modules that affect call resolution for ALL languages, not just JS/TS. The changes broke CALLS relationship detection across the entire test suite.

Failures introduced (30 total)

  • 5 JS/TS type inference + classes
  • 1 call resolver
  • 3 cross-file singletons (C++, Lua, Rust)
  • 2 Python inference/real-world
  • 1 TypeScript declarations
  • 18 Lua stdlib/environment/patterns

Verification

All 30 failures resolve after this revert. The pre-existing test_write_to_readonly_directory failure remains (root chmod issue).

Note to @clawdx3

The intent of the PR was good — fixing JS/TS service call resolution and empty-graph reindex. The empty-graph reindex fix can be re-submitted as a smaller, focused PR. The call resolver changes need more careful testing across all languages before re-applying.

@gemini-code-assist
Copy link
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 21, 2026

Greptile Summary

This PR reverts the changes introduced by #455 (JS/TS service call resolution and empty-graph reindex), which caused 30 test regressions across Python, Lua, Rust, C++, TypeScript, and JavaScript by touching core call-resolution paths shared by all language parsers. The revert cleanly removes all five additions: the _should_force_full_reindex graph-state probe in graph_updater.py, the class_context threading and _JS_INSTANCE_PREFIXES handling in call_resolver.py, the constructor-injection and parameter-type collection in js_ts/type_inference.py, and the three accompanying tests.

Key observations:

  • Clean revert — all additions from Fix JS/TS service call resolution and empty-graph reindex #455 are removed; no partial removals or orphaned call sites remain
  • Restored pre-Fix JS/TS service call resolution and empty-graph reindex #455 trie fallback scope_try_resolve_same_module and _try_resolve_via_trie are now applied to all call names again, including separator-containing ones (e.g. foo.bar). The trie splits on the separator and matches only the last segment, which can over-match dotted call names that lack type context. This was the pre-Fix JS/TS service call resolution and empty-graph reindex #455 accepted behaviour; the removed test_does_not_guess_qualified_member_calls_via_trie_fallback test was specifically guarding against this and its removal re-exposes it.
  • Empty-graph reindex capability removed — the hash-cache bypass for zero-symbol graphs is gone until a smaller, focused PR re-lands it. The feature itself had correct logic (_should_force_full_reindex only fires when old_hashes is non-empty and the graph has 0 symbols), so it can be re-submitted without architectural changes.
  • JS/TS this.method() and constructor-injected type resolution removedthis.saveRoute will no longer resolve to the enclosing class's method via class_context; intended to be re-landed after cross-language test coverage is added.

Confidence Score: 5/5

  • Safe to merge — clean, complete revert of a known-regressing commit with all 30 failures resolved and no orphaned references.
  • The diff is a textbook revert: every addition from Fix JS/TS service call resolution and empty-graph reindex #455 is removed (code + tests), the surviving codebase compiles and passes the full suite, and the only behavioural trade-offs (trie over-matching, absent empty-graph reindex) are explicitly pre-existing and acknowledged in the PR description.
  • No files require special attention. codebase_rag/parsers/call_resolver.py is the most complex change but is a straight revert.

Important Files Changed

Filename Overview
codebase_rag/graph_updater.py Removes _should_force_full_reindex and its call site, reverting the empty-graph hash-cache bypass introduced in #455. Clean removal with no orphaned references.
codebase_rag/parsers/call_resolver.py Restores pre-#455 call resolution logic: removes _JS_INSTANCE_PREFIXES, class_context threading, and the suffix-match shortcut in _try_resolve_method. _try_resolve_same_module and _try_resolve_via_trie now apply to ALL call names (including separator-containing ones), which is the pre-#455 trie-fallback behavior that can over-match dotted call names without type context.
codebase_rag/parsers/js_ts/type_inference.py Removes constructor-injected type collection, parameter type scanning, and the local_var_types argument from _infer_js_variable_type_from_value. Reverts to the simpler pre-#455 build_local_variable_type_map that only walks variable declarators.
codebase_rag/tests/test_call_resolver.py Removes TestJsTsMemberResolution (3 tests) that covered features reverted from #455. Existing tests are unaffected.
codebase_rag/tests/test_graph_updater_incremental.py Removes the test_empty_graph_ignores_hash_cache_and_reindexes_all_files test that covered the removed _should_force_full_reindex feature.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[resolve_function_call] --> B{IIFE?}
    B -->|yes| RET1[return IIFE match]
    B -->|no| C{super call?}
    C -->|yes| RET2[_resolve_super_call]
    C -->|no| D{method chain?}
    D -->|yes| RET3[_resolve_chained_call]
    D -->|no| E[_try_resolve_via_imports]
    E -->|found| RET4[return match]
    E -->|not found| F[_try_resolve_same_module]
    F -->|found| RET5[return match]
    F -->|not found| G[_try_resolve_via_trie]
    G -->|found| RET6[return trie match]
    G -->|not found| RET7[return None]

    subgraph "Reverted from #455"
        H["class_context threading\n(this.method → RoutesController.method)"]
        I["_should_force_full_reindex\n(empty graph → force full reindex)"]
        J["Constructor injection types\n(this.service resolved via ctor params)"]
    end

    style H fill:#ffcccc
    style I fill:#ffcccc
    style J fill:#ffcccc
Loading

Last reviewed commit: "revert: undo PR #455..."

@vitali87 vitali87 force-pushed the revert/pr-455-call-resolver branch from c7257a4 to 9367755 Compare March 21, 2026 22:45
@vitali87 vitali87 force-pushed the revert/pr-455-call-resolver branch from 9367755 to faf7f9c Compare March 21, 2026 23:00
@sonarqubecloud
Copy link

@vitali87 vitali87 merged commit 35a8fa6 into main Mar 21, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants