grounding accuracy MRR@3 0.59 → 0.79, recall 14% → 30%

[silongtan]

Summary

• MRR@3: 0.592 → 0.786 (+32.8%) — measures if correct code is in top-3 results
• Recall: 13.9% → 29.9% (+16pp) — measures if correct symbols are found
• Variance: 0.360 → 0.156 — consistency across repos (Medusa/Saleor/Vendure)
• 49/49 tests pass

Changes

Pipeline improvements (adapters/code_locator.py)

• Two-track token extraction: identifier tokens (camelCase/compound) go through fuzzy matching; raw NL words filtered via keyword blocklist. Prevents "void"→Void,
  "state"→State graph seed pollution while preserving "checkout"→Checkout
• Case-form bigrams: adjacent NL words generate PascalCase/snake_case candidates
• Coverage tiers widened: (2,80,5)→(3,75,5) at Tier 0
• Symbol type priority as Stage 2 tiebreaker

Eval metric improvements (tests/eval_code_locator.py)

• Case-insensitive symbol matching
• All-component extraction from qualified names

Ground truth corrections (tests/fixtures/expected/decisions.py)

• Removed 5 phantom Medusa symbols
• Fixed PluginManager→PluginsManager, on_commit→fulfillment_created
• Fixed overly-specific Vendure file patterns

Config

• BM25 k1/b params now configurable
• Fuzzy scorer configurable (WRatio/token_set_ratio/partial_ratio)

Test plan

• 49/49 unit tests pass
• Eval on 3 OSS repos (30 decisions)
• Saleor MRR stable at 0.864 (no regression)
• Variance under 0.25 CI gate

Summary by CodeRabbit

Release Notes

• New Features

  • Added configurable BM25 tuning parameters for search indexing optimization
  • Introduced selectable fuzzy matching scorer strategies
  • Enhanced symbol ranking with type-priority ordering

• Bug Fixes

  • Improved case-insensitive symbol matching and detection
  • Refined tokenization for better identifier recognition and relevance filtering

[coderabbitai]

📝 Walkthrough

Walkthrough

Configuration and algorithm tuning across code locator's grounding and retrieval systems: adjusted coverage-tier limits (max_files increased 2→3, 4→5, 6→7) and fuzzy thresholds, added symbol-type-priority ranking, expanded tokenization with regex-based identifier detection, exposed BM25 hyperparameters (k1, b) and RRF parameter (rrf_k) as configurable options, added pluggable fuzzy scorer selection, and made test symbol matching case-insensitive with corresponding fixture and assertion updates.

Changes

Cohort / File(s)	Summary
Configuration & Runtime code_locator/config.py, code_locator_runtime.py	Added BM25 tuning parameters bm25_k1 (1.5) and bm25_b (0.75); adjusted RRF parameter rrf_k default from 60 to 40. Updated rebuild_index() to explicitly pass these hyperparameters to BM25 initialization.
Symbol Grounding & Ranking adapters/code_locator.py	Tightened coverage-tier thresholds (max_files: 2→3, 4→5, 6→7; fuzzy scores: 80→75, 70→65, 60→55). Added _SYMBOL_TYPE_PRIORITY mapping with _type_priority() helper; extended per-file symbol ranking to include symbol type priority as third sort key. Expanded _ground_single tokenization with regex-based identifier detection, keyword filtering, and three-track token generation (compound/camelCase identifiers, domain words, and case-form bigrams).
BM25 Retrieval code_locator/retrieval/bm25s_client.py	Updated index() method signature to accept optional k1 and b BM25 tuning parameters (defaults 1.5 and 0.75); passed parameters to bm25s.BM25 constructor.
Fuzzy Matching code_locator/tools/validate_symbols.py	Refactored Stage 2 of _fuzzy_match to select scorer dynamically from config (WRatio, token_set_ratio, or partial_ratio), defaulting to WRatio. Score now computes maximum of selected scorer's outputs for both name and qualified_name.
Test Evaluation & Fixtures tests/eval_code_locator.py, tests/fixtures/expected/decisions.py	Made symbol matching case-insensitive in _is_relevant() and recall computation; expanded found_symbols collection to include all dot-separated symbol parts. Updated decision fixture expectations for symbol mappings (removed CartCompletionStrategy, JobSchedulerService, PaymentSessionService, PluginManager, WebhookEndpoint; renamed PluginManager → PluginsManager; adjusted file pattern matching for Vendure fixtures).
Test Coverage Loop tests/test_coverage_loop.py	Updated tier progression expectations: strict tier 0 now triggers at max_f == 3 (was 2), tier 1 at max_f == 5 (was 4), full sequence [3, 5, 7] (was [2, 4, 6]). Adjusted all related assertions for tiers_tried and tier stamping tests.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~35 minutes

Possibly related PRs

• fix: compound token extraction for grounding recall (9%→14%) #26 — Directly extends identifier-token extraction in _ground_single tokenization with expanded regex-based detection and multi-track token generation.
• fix: Stage 1 symbol ranking by fuzzy score — MRR@3 0.39→0.58  #25 — Modifies symbol ranking logic in _ground_single to incorporate fuzzy match scores, complementing the new symbol-type-priority tiebreak.
• decision grounding reuse + coverage loop #5 — Adjusts grounding/coverage loop tier thresholds and _ground_single behavior, overlapping with tier max_files and tokenization changes.

Suggested reviewers

• jinhongkuan

Poem

🐰 Hops through the thresholds, tightening the knot,
Three token tracks dance where once was just thought,
BM25's parameters now tuned just right,
Symbols ranked by type—what a glorious sight!
Case-insensitive matching, fuzzy and fleet,
Config and testing complete the retreat. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 68.42% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly reflects the main achievement of the PR: significant improvements in grounding accuracy metrics (MRR@3 from 0.59 to 0.79, recall from 14% to 30%), which aligns with the PR objectives that demonstrate these exact metric improvements.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch silong/grounding-accuracy-improvements

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (6)

tests/eval_code_locator.py (1)

129-133: Minor consistency nit: denominator could use lowercased set for symmetry.

If expected_symbols ever contains case-variant duplicates (e.g., {"Foo", "foo"}) — unlikely with the current fixtures — the numerator collapses after .lower() while the denominator uses the raw set cardinality, slightly underreporting recall. Using len(expected_lower) would make the metric consistent with the case-insensitive intent. Not a bug with the current fixture contents; flagging only for robustness.

♻️ Optional tweak

-        recall = matched_count / len(expected_symbols) if expected_symbols else 0
+        recall = matched_count / len(expected_lower) if expected_lower else 0

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/eval_code_locator.py` around lines 129 - 133, The recall calculation
uses len(expected_symbols) as the denominator while the numerator is derived
from expected_lower (case-folded), which miscounts when expected_symbols has
case-variant duplicates; update the recall computation to use
len(expected_lower) (i.e., recall = matched_count / len(expected_lower) if
expected_lower else 0) so the denominator matches the case-insensitive sets used
to compute matched_count (references: expected_lower, found_lower,
matched_count, recall).

adapters/code_locator.py (3)

152-155: Annotate _SYMBOL_TYPE_PRIORITY as ClassVar (RUF012).

Ruff flags this as a mutable class attribute. The mapping is effectively constant; annotating with ClassVar documents that intent and silences the lint without behavior change.

♻️ Proposed annotation

+from typing import ClassVar
@@
-    _SYMBOL_TYPE_PRIORITY = {
+    _SYMBOL_TYPE_PRIORITY: ClassVar[dict[str, int]] = {
         "class": 0, "interface": 1, "type_alias": 2,
         "function": 3, "method": 4, "variable": 5,
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@adapters/code_locator.py` around lines 152 - 155, Annotate the class-level
mapping _SYMBOL_TYPE_PRIORITY with ClassVar to mark it as an immutable class
attribute: add "from typing import ClassVar" to imports (or include ClassVar in
the existing typing import) and change the declaration to use ClassVar[dict[str,
int]] (or ClassVar[Mapping[str, int]] if Mapping is preferred) for the variable
type; keep the existing mapping values and name _SYMBOL_TYPE_PRIORITY unchanged
so the lint (RUF012) is satisfied without behavioral change.

---

208-210: Dead helper — _type_priority is defined but never called.

The sort lambda at lines 351-355 inlines self._SYMBOL_TYPE_PRIORITY.get(r["type"], 3) directly (which is correct, since the row is already in hand — calling this helper would add an unnecessary db.lookup_by_id per symbol). Either remove the helper or route the sort key through it; leaving it as-is invites future callers to pick the DB-fetching version by mistake.

♻️ Suggested removal

-    def _type_priority(self, sid: int, db) -> int:
-        row = db.lookup_by_id(sid)
-        return self._SYMBOL_TYPE_PRIORITY.get(row["type"], 3) if row else 3
-

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@adapters/code_locator.py` around lines 208 - 210, _remove the dead helper
_type_priority which performs an unnecessary db lookup; delete the method
definition and any unused imports so callers use the inline expression
self._SYMBOL_TYPE_PRIORITY.get(r["type"], 3) (or, if you intended a reusable
helper, replace _type_priority(sid, db) with a non-DB version like
_type_priority_from_row(row) that reads row["type"]). Ensure no remaining
references to _type_priority exist and adjust tests/usages accordingly.

---

234-267: Tokenization looks correct — one small heads-up on acronym handling.

Three-track extraction with de-dup is clean. One heuristic wrinkle: str.capitalize() lowercases the tail, so acronyms from the description — "JWT permissions" — produce "JwtPermissions" / "jwt_permissions" rather than "JWTPermissions". That's still a plausible symbol candidate but won't match true all-caps acronym-prefixed class names like JWTHandler. If grounding acronyms becomes a recall gap in follow-up evals, consider preserving the original casing of already-upper tokens (e.g., w if w.isupper() else w.capitalize()). Not blocking.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@adapters/code_locator.py` around lines 234 - 267, The pascal-case
construction in the case_forms loop lowercases acronym tails via
str.capitalize(), causing all-caps tokens like "JWT" to become "JwtPermissions"
and miss true symbols like "JWTPermissions"; update the loop in
adapters/code_locator.py (the nl_words → case_forms section) to preserve
already-uppercase words when building pascal (e.g., use the original token if
token.isupper() else token.capitalize()) and similarly ensure snake uses
lowercased tokens for separation; keep the other heuristics and the length check
intact.

code_locator/retrieval/bm25s_client.py (1)

68-68: Protocol and concrete-class signatures now diverge — consider keeping them in sync.

BM25Search.index(self, repo_path, output_dir) (per code_locator/retrieval/bm25_protocol.py) no longer matches this concrete implementation, which now also accepts symbol_db, k1, and b. Because all extras have defaults, callers using the protocol type still work, so this is cosmetic — but documenting the tuning knobs at the protocol level would help future BM25 backends (e.g. the mentioned "zoekt") honor them.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@code_locator/retrieval/bm25s_client.py` at line 68, The BM25 protocol
signature and docs need to match the concrete implementation: update
BM25Search.index in bm25_protocol.py to accept the same optional parameters
(symbol_db=None, k1: float = 1.5, b: float = 0.75) and add a short docstring
describing each tuning knob so other backends (e.g., zoekt) can implement them
consistently; ensure the parameter names and default values exactly match the
concrete method index in bm25s_client.py.

code_locator/tools/validate_symbols.py (1)

80-85: Hoist _SCORERS to module or class scope.

The dict is rebuilt on every call to _fuzzy_match. Lifting it to a module-level constant (or class attribute) keeps the scorer resolution centralized and avoids the per-call dict allocation. Functionally fine as-is.

♻️ Proposed refactor

 from rapidfuzz import fuzz

+_SCORERS = {
+    "WRatio": fuzz.WRatio,
+    "token_set_ratio": fuzz.token_set_ratio,
+    "partial_ratio": fuzz.partial_ratio,
+}
+
 # JSON Schema for tool parameter validation
@@
-        _SCORERS = {
-            "WRatio": fuzz.WRatio,
-            "token_set_ratio": fuzz.token_set_ratio,
-            "partial_ratio": fuzz.partial_ratio,
-        }
-        scorer = _SCORERS.get(self.config.fuzzy_scorer, fuzz.WRatio)
+        scorer = _SCORERS.get(self.config.fuzzy_scorer, fuzz.WRatio)

One silent-fallback concern worth considering: an unknown/typo'd fuzzy_scorer value in config currently silently reverts to WRatio. Consider logging a warning so a misconfigured env var (CODE_LOCATOR_FUZZY_SCORER=token_sort_ratio) doesn't quietly run with the wrong scorer.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@code_locator/tools/validate_symbols.py` around lines 80 - 85, Hoist the
_SCORERS dict out of _fuzzy_match into module scope (e.g. TOP_LEVEL _SCORERS
constant) or a class attribute so it isn’t recreated per call, then have
_fuzzy_match look up scorer = _SCORERS.get(self.config.fuzzy_scorer,
fuzz.WRatio); additionally, detect when the get() falls back (i.e.
self.config.fuzzy_scorer not a key) and emit a warning via the existing logger
(or logging.getLogger(__name__)) indicating the unknown fuzzy_scorer and that
WRatio is being used as a fallback; keep symbol names _SCORERS, _fuzzy_match,
and self.config.fuzzy_scorer exactly as in the diff so the changes are easy to
locate.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@adapters/code_locator.py`:
- Around line 152-155: Annotate the class-level mapping _SYMBOL_TYPE_PRIORITY
with ClassVar to mark it as an immutable class attribute: add "from typing
import ClassVar" to imports (or include ClassVar in the existing typing import)
and change the declaration to use ClassVar[dict[str, int]] (or
ClassVar[Mapping[str, int]] if Mapping is preferred) for the variable type; keep
the existing mapping values and name _SYMBOL_TYPE_PRIORITY unchanged so the lint
(RUF012) is satisfied without behavioral change.
- Around line 208-210: _remove the dead helper _type_priority which performs an
unnecessary db lookup; delete the method definition and any unused imports so
callers use the inline expression self._SYMBOL_TYPE_PRIORITY.get(r["type"], 3)
(or, if you intended a reusable helper, replace _type_priority(sid, db) with a
non-DB version like _type_priority_from_row(row) that reads row["type"]). Ensure
no remaining references to _type_priority exist and adjust tests/usages
accordingly.
- Around line 234-267: The pascal-case construction in the case_forms loop
lowercases acronym tails via str.capitalize(), causing all-caps tokens like
"JWT" to become "JwtPermissions" and miss true symbols like "JWTPermissions";
update the loop in adapters/code_locator.py (the nl_words → case_forms section)
to preserve already-uppercase words when building pascal (e.g., use the original
token if token.isupper() else token.capitalize()) and similarly ensure snake
uses lowercased tokens for separation; keep the other heuristics and the length
check intact.

In `@code_locator/retrieval/bm25s_client.py`:
- Line 68: The BM25 protocol signature and docs need to match the concrete
implementation: update BM25Search.index in bm25_protocol.py to accept the same
optional parameters (symbol_db=None, k1: float = 1.5, b: float = 0.75) and add a
short docstring describing each tuning knob so other backends (e.g., zoekt) can
implement them consistently; ensure the parameter names and default values
exactly match the concrete method index in bm25s_client.py.

In `@code_locator/tools/validate_symbols.py`:
- Around line 80-85: Hoist the _SCORERS dict out of _fuzzy_match into module
scope (e.g. TOP_LEVEL _SCORERS constant) or a class attribute so it isn’t
recreated per call, then have _fuzzy_match look up scorer =
_SCORERS.get(self.config.fuzzy_scorer, fuzz.WRatio); additionally, detect when
the get() falls back (i.e. self.config.fuzzy_scorer not a key) and emit a
warning via the existing logger (or logging.getLogger(__name__)) indicating the
unknown fuzzy_scorer and that WRatio is being used as a fallback; keep symbol
names _SCORERS, _fuzzy_match, and self.config.fuzzy_scorer exactly as in the
diff so the changes are easy to locate.

In `@tests/eval_code_locator.py`:
- Around line 129-133: The recall calculation uses len(expected_symbols) as the
denominator while the numerator is derived from expected_lower (case-folded),
which miscounts when expected_symbols has case-variant duplicates; update the
recall computation to use len(expected_lower) (i.e., recall = matched_count /
len(expected_lower) if expected_lower else 0) so the denominator matches the
case-insensitive sets used to compute matched_count (references: expected_lower,
found_lower, matched_count, recall).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6bb0a65c-4ad5-4cb6-a0b7-7154001e85d6

📥 Commits

Reviewing files that changed from the base of the PR and between f06b5df and 6e466f1.

📒 Files selected for processing (8)

• adapters/code_locator.py
• code_locator/config.py
• code_locator/retrieval/bm25s_client.py
• code_locator/tools/validate_symbols.py
• code_locator_runtime.py
• tests/eval_code_locator.py
• tests/fixtures/expected/decisions.py
• tests/test_coverage_loop.py