Skip to content

fix(incremental): keep interface implementers of unchanged files - #3299

Merged
vitali87 merged 5 commits into
mainfrom
claude/java-interface-implementers-rehydrate-3256
Oct 18, 2026
Merged

vitali87 merged 5 commits into
mainfrom
claude/java-interface-implementers-rehydrate-3256

Conversation

@vitali87

@vitali87 vitali87 commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Problem: in Java, a call typed to an interface with a single first-party implementer also gets CALLS edges to the implementation and each of its @line variants, such as enum-constant bodies (_emit_interface_sole_impls). A fresh index emits them. After any incremental sync that re-parses the caller's file but not the implementer's, they were gone. On google/gson, a 40-commit replay lost 8 edges, among them getFieldNames → FieldNamingPolicy.translateName and its six constant overrides.
  • A second, worse case found while testing: when a new implementer was added in the same run, the stored one didn't count. The new class then looked like the sole implementer, and the call gained a wrong edge to it that a fresh index never produces.
  • Cause: both inputs to the fan-out were filled only by classes ingested in the current run, and nothing rehydrated them:
    • interface_implementers, which says whether an interface has a sole implementer;
    • the registry's duplicate buckets, which give variants() the @line bodies.
  • Fix:
    • _rehydrate_interface_implementers_from_graph, run beside _rehydrate_class_inheritance_from_graph, reads the project's IMPLEMENTS edges (CYPHER_ALL_IMPLEMENTS) into interface_implementers. A re-parsed file's classes were deleted with their old edges, so a dropped implements isn't read back.
    • FunctionRegistryTrie.restore_variant(qn) puts a rehydrated natural@line (or @line_col) qn back in its natural qn's bucket. _rehydrate_definition_row calls it for every row it restores; any other qn is left alone.
    • The eval graph double answers the new query.

Demo

Java sole-implementer fan-out across an incremental sync: main vs this branch

The GIF plays the issue's ji repro step by step against Memgraph.

  • Repo: enum Policy implements Strategy has name(String) and two constant-body overrides, and Factory.names calls strategy.name(f).
  • Steps: a fresh index, callees Factory.names, a comment-only edit to Factory.java committed, an incremental cgr start --update-graph, and callees again.
  • main: fresh Policy.name(String), @11, @8 and Strategy.name(String); after the sync, Strategy.name(String) only.
  • This branch: all four, before and after the sync. The new tests pass.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement

Related Issues

Fixes #3256

Test Plan

  • All existing unit tests pass (make test): related suites run, full suite see below
  • New tests added for new functionality
  • Integration tests pass (if applicable)
  • Manual testing performed

Red/green TDD in codebase_rag/tests/test_incremental_interface_implementers.py: a fresh index and then an incremental run, through the stateful graph double.

  • Red on main (87a8f58): 3 failed, 1 passed.
    • test_a_reparsed_caller_keeps_its_sole_implementer_edges: the issue's repro, with all four callees after an edit to Factory.java alone.
    • test_a_second_implementer_ends_the_sole_fan_out: Other implements Strategy is added while Policy is unchanged. On main the call gains a wrong edge to Other.name; it must match a fresh index of the same tree.
    • test_restore_variant_reads_only_a_line_or_line_column_suffix, a unit test.
  • Green on this branch: 4 passed.
  • Negative (passes on main and here): test_an_implementer_that_drops_the_interface_loses_its_edges. Policy re-parsed without implements Strategy gives what a fresh index gives: the interface method only.
  • Mutation checks: no implementer rehydration and no variant restore are both caught. A path-based skip of re-parsed implementers was removed: re-parsing deletes those classes' old edges, so it had no observable effect.
  • Wider runs:
    • Every test_java*, test_jvm*, test_incremental*, test_reingest*, test_function_registry*, test_registry*, test_graph_updater*, test_eval_double*, test_interface*, test_implements*, test_rehydrat* and test_duplicate* file (114 files): 1,236 passed, 1 skipped.
    • ruff check, ruff format and ty are clean.
  • Note: fix(incremental): index a rehydrated definition under its name #3296 ([Bug]: incremental sync: C# methods with parameters in unchanged files are invisible to bare-name resolution (rehydration indexes ILog.Info(string) under Info(string), not Info), so a re-parsed caller binds l.Info("hi") to an unrelated Levels.Info() (serilog replay: all 33 incremental-only heuristic edges) #3261) edits the same lines of _rehydrate_definition_row; whichever merges second keeps both calls.
    • Full suite with Memgraph (local, -n 4): 109 failed, 17,712 passed, 176 skipped. The 109 failures are the same set main fails in this environment (LLM provider keys, the build-binary check, the unreachable-network tests); none is new.
  • Manual: the demo above.

Checklist

  • My code follows the Conventional Commits format
  • I have run pre-commit hooks locally: ruff, ruff format and ty were run by hand on the changed files
  • No hardcoded strings (all in constants or StrEnum)
  • No # type: ignore, cast(), Any, or object type hints
  • Comments explain why, not what

🤖 Generated with Claude Code

https://claude.ai/code/session_01EbsCWQcSTGQXv3g5sk418m


Generated by Claude Code

Summary by CodeRabbit

  • Tests
    • Expanded coverage for incremental updates when enum implementations change, including removal of an implementation and addition of another.
    • Added checks that incremental results match a fresh index after related code changes.
    • Broadened variant-name tests to confirm names with non-numeric suffixes remain distinct.

A call typed to an interface with one first-party implementer also
binds the implementation and each of its `@line` variants (enum
constant bodies), read from `interface_implementers` and the registry's
duplicate buckets. Both were filled only by the classes parsed in the
current run, so a comment edit to the caller dropped the fan-out (gson:
8 edges), and beside a new implementer the stored one did not count, so
the new one looked sole.

The run now reads the project's IMPLEMENTS edges back into
`interface_implementers` beside the INHERITS rehydration, and a
rehydrated `natural@line` qn rejoins its natural qn's variant bucket.

Fixes #3256

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EbsCWQcSTGQXv3g5sk418m
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T20:03:02.287939Z 89a4b22 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: vitali87/code-graph-rag/.coderabbit.yaml
  • Review profile: QUIET
  • Plan: Advanced
  • Run ID: 221681c9-e46e-4fdf-ba08-57cbe610b5b3
📥 Commits

Reviewing files that changed from the base of the PR and between 89a4b22 and 314e35f.

📒 Files selected for processing: 2
  • codebase_rag/tests/test_incremental_duplicate_variants.py
  • codebase_rag/tests/test_incremental_interface_implementers.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The pull request adds regression tests for enum implementer fan-out across sync modes and for names with non-numeric trailing suffixes. The tests compare incremental results with a fresh index or confirm that a name remains its own variant.

Changes: Enum implementer fan-out tests

Layer / File(s) Summary
Check enum implementer fan-out during sync
codebase_rag/tests/test_incremental_interface_implementers.py
Adds sync cases that remove the enum’s interface or add a second implementer. The tests compare caller edges and graph snapshots with a fresh index. They check that only the interface method remains among the caller’s calls.

Duplicate variant suffix tests

Layer / File(s) Summary
Cover a non-numeric trailing suffix
codebase_rag/tests/test_incremental_duplicate_variants.py
Adds p.C.o@x to the parameterized test and checks that it remains its own sole variant.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 314e3

This change adds regression tests for incremental Java interface-implementer handling and for variant-name parsing. No concrete merge-blocking risk was identified in the supplied context.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title is concise, uses Conventional Commits format, and accurately identifies the main fix: preserving interface implementers during incremental indexing.
Description check Passed The description includes all required sections and provides clear problem, fix, demo, change type, issue link, test plan, validation results, and checklist details. The unchecked pre-commit item is cl…
Linked Issues check Passed The reviewed head satisfies the coding requirements in issue #3256. Existing regression coverage compares incremental and fresh graphs for caller-only edits, including enum constant-body variants. The…
Out of Scope Changes check Passed The changes stay within issue #3256. The added cases validate stale implementer removal, multiple implementers, and variant cleanup. The non-numeric variant test validates the registry rule used by th…
Full details: Description check

Explanation

The description includes all required sections and provides clear problem, fix, demo, change type, issue link, test plan, validation results, and checklist details. The unchecked pre-commit item is clarified by the note that ruff, ruff format, and ty were run manually.

Full details: Linked Issues check

Explanation

The reviewed head satisfies the coding requirements in issue #3256. Existing regression coverage compares incremental and fresh graphs for caller-only edits, including enum constant-body variants. The added parameterized test also verifies that a removed implements relation and a second implementer retire the sole-implementer fan-out. The whole-PR diff contains these focused tests, and the reviewed head contains the corresponding rehydration behavior.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #3256. The added cases validate stale implementer removal, multiple implementers, and variant cleanup. The non-numeric variant test validates the registry rule used by the same incremental fan-out fix. No unrelated product behavior or high-severity change is shown.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 89a4b22cac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codebase_rag/graph_updater.py Resolved
@greptile-apps

greptile-apps Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] No blocking issues found; the checked incremental-indexing scenarios preserve the expected call targets and respect removed interface relationships.

T-Rex evidence

Command output from the check

  • The requested test command initially exited 127 because `.venv/bin/python` was unavailable, showing an environment blocker subsequently resolved.

Command output from the check

  • The setup-prescribed frozen uv sync command exited 0, restoring the environment needed to execute the requested tests.

Evidence from the check

  • This file contains the exact pytest command executed with the installed virtualenv Python, making the targeted test run reproducible.

Command output from the check

  • The two named existing tests ran successfully with exit code 0 and two passes, rejecting the hypothesis within their stateful-double coverage.

Evidence from the check

  • The executed Python harness prints implementer registries, stored IMPLEMENTS rows, and caller targets for addition and removal scenarios, exposing the behavior directly.

Evidence from the check

  • The executed shell script runs both harness modes and records each command, working directory, exit code, and observed output, preserving reproducible evidence.

Command output from the check

  • The control run records an erroneous Other implementation target despite two stored implementers while interface removal remains correct, demonstrating that the check detects the hypothesized failure.

Command output from the check

  • The PR run records both implementers in the canonical registry and only the interface CALLS target, while removal leaves no stored IMPLEMENTS rows, confirming correct behavior in the tested scenarios.

Evidence from the check

  • The executed script runs the two named pytest tests and captures fresh and incremental call targets using the existing stateful graph double, making the narrow check reproducible.

Command output from the check

  • Ran both named tests with installed .venv Python and captured pytest output, showing two passes and exit code 0.

Command output from the check

  • Indexed the Java fixture from scratch and printed its four call targets, establishing that both enum-constant implementation edges exist before the edit.

Command output from the check

  • Applied a comment-only change to Factory.java and indexed incrementally against the same in-process store, showing that all four targets remain unchanged.

View artifacts

Summary

This PR keeps interface implementers and duplicate method variants available when an incremental run leaves their files unchanged.

  • Incremental interface calls keep the right concrete targets across updates.
  • Stored duplicate methods join interface-call fan-out again.

No blocking issues found in the checked behavior. Execution checks used real Java parsing with the in-process stateful graph double; live Memgraph execution was not verified.

Reviews (1) · Last reviewed commit: "fix(incremental): keep interface impleme..." · Reviewed by Greptile

…ehydrate-3256

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016xpVRsxnpLfjsPc7qrjoyy

# Conflicts:
#	codebase_rag/constants/graph.py
#	codebase_rag/function_registry.py
#	codebase_rag/graph_updater.py
#	codebase_rag/tests/test_incremental_interface_implementers.py
…ehydrate-3256

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016xpVRsxnpLfjsPc7qrjoyy
No conflicts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013MgsnsA3jM14E4ai22xnhy
@sonarqubecloud

Copy link
Copy Markdown

@vitali87
vitali87 merged commit 23ffb5c into main Oct 18, 2026
40 checks passed
@vitali87
vitali87 deleted the claude/java-interface-implementers-rehydrate-3256 branch October 18, 2026 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

2 participants