Skip to content

fix(graph): restore linear-scan fallback in find_ending_with#557

Open
ChetanyaRathi wants to merge 1 commit into
vitali87:mainfrom
ChetanyaRathi:fix/find-ending-with-fallback-513
Open

fix(graph): restore linear-scan fallback in find_ending_with#557
ChetanyaRathi wants to merge 1 commit into
vitali87:mainfrom
ChetanyaRathi:fix/find-ending-with-fallback-513

Conversation

@ChetanyaRathi

@ChetanyaRathi ChetanyaRathi commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a bug where FunctionRegistryTrie.find_ending_with() silently dropped call-resolution results when _simple_name_lookup was populated but did not contain the searched suffix.

The method returned an empty result in that case instead of falling through to the linear scan over _entries, so any qualified name ending in .<suffix> was lost. This affects repos where a function is registered in the trie but its simple name is not indexed in _simple_name_lookup (generated names, IIFE patterns, some JS/TS module edge cases).

Root cause

find_ending_with has two paths: a fast path over _simple_name_lookup and a
linear scan over _entries. _simple_name_lookup is optional — it is passed in
at construction and defaults to None (graph_updater.py:64-65), and when
present it is kept in sync by insert itself (graph_updater.py:117-118). So the
two failure-free cases are: lookup is None (scan runs) and lookup is present
and complete (fast path hits).

The pre-fix bug surfaced only in a third state — a lookup that is present but
does not contain the queried suffix. In that case the old code returned []
instead of falling back to the _entries scan. The fix restores the fallback so
a present-but-incomplete lookup degrades to the scan rather than dropping the
result.

Note: because insert maintains a present lookup, this incomplete-lookup state
is not produced by insert alone; it requires the lookup to be populated by a
separate path. I verified the fallback behavior directly via the added unit
test, but I have not identified that construction path in the current tree, so
I'm treating the axios impact figure as reported in #513 rather than one I
traced.

Impact

14 CALLS relationships were silently dropped on the axios repo (JavaScript). Any repo whose functions are registered in the trie without their simple names being indexed in _simple_name_lookup was affected.

How it was found

Surfaced by expanding the benchmark suite from 2 repos (requests, tree-sitter) to 10 repos across the supported languages — the original two never exercised the failing path, but axios did.

Test plan

Added a regression test (test_find_ending_with_falls_back_when_suffix_missing_from_lookup) covering all three branches: _simple_name_lookup is None, lookup present and containing the suffix, and lookup present but missing the suffix (the bug state). Verified it fails on the pre-fix code and passes with the fix. Full test_trie_optimization.py suite passes.

Fixes #513

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the find_ending_with method in codebase_rag/graph_updater.py to ensure that if a suffix is not found in _simple_name_lookup, the method correctly falls back to scanning all entries instead of returning an empty list. A new unit test has been added in codebase_rag/tests/test_trie_optimization.py to verify this fallback behavior. There are no review comments, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@greptile-apps

greptile-apps Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes FunctionRegistryTrie.find_ending_with so lookup misses still use the full registry fallback. The main changes are:

  • Collapses the lookup branch so only actual _simple_name_lookup hits skip the scan.
  • Restores linear scanning over _entries when the lookup is absent or missing the suffix.
  • Adds tests for no lookup, lookup hit, and lookup-miss fallback behavior.

Confidence Score: 5/5

The change is narrowly scoped to restoring the intended fallback path and is covered by focused regression tests.

The updated conditional preserves the optimized lookup-hit behavior while allowing misses to use the existing full registry scan, and the tests cover the affected branches.

T-Rex T-Rex Logs

What T-Rex did

  • We observed the base behavior before the artifact, where the info and debug branches succeeded and the present-but-missing lookup warn branch returned [].
  • We observed the head behavior after the artifact, where the info and debug branches still succeeded and the present-but-missing lookup warn branch returned ['com.example.Logger.warn'].

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "fix(graph): restore linear-scan fallback..." | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

T-Rex pricing update — T-Rex was free through June 2026. Effective July 1, 2026, T-Rex adds 2 credits on top of the standard 1-credit review (3 total). T-Rex settings

@ChetanyaRathi

Copy link
Copy Markdown
Contributor Author

The failing CI checks all error within ~2s, which looks like a shared setup
step failing rather than anything in this diff — the unit-test jobs don't run
long enough to have executed the suite, and the PR-title check is failing on a
title that matches the convention. On my end ruff and the
test_trie_optimization.py suite pass on the two changed files, and the T-Rex
review verified the fix by running the code. Happy to rebase onto latest main
if that would help CI go green.

@vitali87

vitali87 commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Non-blocking: since FunctionRegistryTrie.insert always adds the simple name to the shared simple_name_lookup, the "registered in the trie but simple name not indexed" state is not reachable via insert alone, so could you cite the parser path that actually dropped the 14 axios CALLS (likely a direct dict mutation in js_ts/ingest.py)? The fix is correct either way; just want the root-cause note to match reality.

@vitali87 vitali87 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read my comment aboave

@ChetanyaRathi

Copy link
Copy Markdown
Contributor Author

Traced this in graph_updater.py. The divergence isn't a parser mutating the
trie directly, I don't see any bulk ⁠ _entries[...] ⁠ write or ⁠ .insert() ⁠ call
under parsers/. It's narrower: insert only maintains the lookup when it's
non-None (graph_updater.py:117-118), and _simple_name_lookup is supplied at
construction (line 65) rather than always derived from _entries. So when a trie
holds names whose simple names aren't in the separately-populated lookup,
find_ending_with took the "lookup present" branch and returned [] instead of
falling back to the _entries scan, which the fix restores.

I've updated the root-cause note to reflect that. I verified the fallback
behavior itself via the direct-state unit test, but I haven't traced the exact
axios construction path that produced the 14-CALL drop, the impact figure is
from the issue. If you know the specific call site that builds the trie with the
mismatched lookup on the JS/TS path, happy to reference it precisely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

fix: find_ending_with drops CALLS when suffix not in simple_name_lookup

2 participants