SED-4340 find-usages-of-keyword-and-plan-referenced-by-attribute-is-b… #579

david-stephan · 2026-01-08T13:25:22Z

…roken

gemini-code-assist · 2026-01-08T13:25:58Z

Summary of Changes

Hello @david-stephan, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a defect in the 'find usages' functionality, specifically targeting issues where keywords and plans referenced by attributes were not correctly identified. The changes involve a significant refactoring of the reference finding mechanism, moving the core logic into a new, dedicated class and enhancing the underlying entity dependency traversal. This ensures that the system can now accurately and comprehensively resolve all potential references, improving the reliability of usage analysis for dynamically referenced entities.

Highlights

New ReferenceFinder Class: Introduced a dedicated ReferenceFinder class to centralize and encapsulate the logic for finding references to plans, functions (keywords), and resources. This promotes better separation of concerns.
Refactored ReferenceFinderServices: The ReferenceFinderServices class has been refactored to delegate its core reference-finding functionality to the new ReferenceFinder, simplifying the REST endpoint implementation.
Enhanced Entity Dependency Tree Traversal: The EntityDependencyTreeVisitor now uses a VISIT_MODE enum (SINGLE, RECURSIVE, RESOLVE_ALL) instead of a boolean flag, providing more granular control over how entity dependencies are traversed and resolved. The RESOLVE_ALL mode is crucial for finding all references.
Improved Plan and Function Entity Resolution: Updated PlanEntity and FunctionEntity to correctly resolve all matching plans and functions, respectively, when the EntityDependencyTreeVisitor is in RESOLVE_ALL mode. This specifically addresses issues with attribute-based lookups.
Flexible Locator Helper: The FunctionLocator and PlanLocator now support a non-strict resolution mode, allowing them to find all potential matches for entities, even if dynamic selection criteria are unresolvable or activation expressions are bypassed.
Comprehensive Unit Tests: Added new unit tests for the ReferenceFinder to ensure the correct identification of references for various entity types and lookup methods (by ID and by name).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a significant and well-executed refactoring of the reference finding logic to address an issue with locating usages of keywords and plans referenced by attributes. The key improvements include extracting the core logic into a dedicated ReferenceFinder class, introducing a more flexible VISIT_MODE for dependency traversal, and enhancing the PlanLocator and FunctionLocator to support a non-strict resolution mode. This non-strict mode is crucial for finding all potential references without a full execution context. The addition of a comprehensive test suite for the new ReferenceFinder is also a great enhancement.

My review includes a high-severity bug fix for a potential NullPointerException and a couple of medium-severity suggestions to further improve code maintainability and test quality. Overall, these are excellent changes that effectively solve the underlying problem.

step-controller/step-controller-server/src/main/java/step/core/references/ReferenceFinder.java

gemini-code-assist · 2026-01-08T13:28:13Z

step-controller/step-controller-server/src/main/java/step/core/references/ReferenceFinder.java

+        List<Object> referencedObjects = getReferencedObjects(entityType, object).stream().filter(o -> (o != null &&  !o.equals(object))).collect(Collectors.toList());
+        //System.err.println("objects referenced from plan: " + planToString(plan) + ": "+ referencedObjects.stream().map(ReferenceFinderServices::objectToString).collect(Collectors.toList()));
+        return referencedObjects.stream().filter(o -> doesRequestMatch(request, o)).collect(Collectors.toList());


This method can be simplified into a single, more efficient stream pipeline that avoids creating an intermediate list. Additionally, there's a commented-out debug line that should be removed as it's no longer relevant and references code that has been moved.

return getReferencedObjects(entityType, object).stream() .filter(o -> o != null && !o.equals(object)) .filter(o -> doesRequestMatch(request, o)) .collect(Collectors.toList());

...ontroller/step-controller-server/src/test/java/step/core/references/ReferenceFinderTest.java

jeromecomte

The changes look good but I indeed see a risk in the changes related to the FunctionLocator and PlanLocator. The new JUnit test ReferenceFinderTest looks good but I'm not sure if it covers all the aspects of the Locators, especially the ones related to versions. In order to merge this to 29, we should ensure that we cover this properly and add some tests if required

jeromecomte · 2026-01-09T08:27:34Z

step-core/src/main/java/step/core/entities/EntityDependencyTreeVisitor.java

 	// TODO declare it as non-static to avoid potential leaks
 	private static final Map<Class<?>, BeanInfo> beanInfoCache = new ConcurrentHashMap<>();

+	public enum VISIT_MODE {


Please add a javadoc to document the exact purpose of these 3 modes

step-controller/step-controller-server/src/main/java/step/core/references/ReferenceFinder.java

jeromecomte · 2026-01-09T09:22:08Z

step-plans/step-plans-base-artefacts/src/main/java/step/artefacts/handlers/FunctionLocator.java

 			} else {
-				// No active versions defined. Return the first function
-				function = orderedFunctions.stream().findFirst().orElseThrow(() -> new NoSuchElementException("Unable to find keyword with attributes " + selectionAttributesJson));
+				//No version defined with simply return the ordered function by priorities


COmment seems not correct

david-stephan

Thanks for the review, all points should be addressed with my last commits.

step-controller/step-controller-server/src/main/java/step/core/references/ReferenceFinder.java

jeromecomte · 2026-01-09T11:39:05Z

step-controller/step-controller-server/src/main/java/step/core/references/ReferenceFinder.java

+                        results.add(new FindReferencesResponse(function));
+                    }
+                } catch (Exception e) {
+                    logger.error("Unable to find references for function {}", function.getId(), e);


SHouldn't we rethrow this or add the error to the response?

Not sure how relevant that is. The implementation will browse all plans and all composite keywords to see if they use the searched reference, if an exception is raised because the context could not be rebuilt for one of them, I don't think this should break the operation, adding it as a warning would be doable (with quite some effort) but I also don't think this warning would be relevant for the user doing the request.

jeromecomte · 2026-01-09T11:39:51Z

step-controller/step-controller-server/src/main/java/step/core/references/ReferenceFinder.java

+                        results.add(new FindReferencesResponse(plan));
+                    }
+                } catch (Exception e) {
+                    logger.error("Unable to find references for plan {}", plan.getId(), e);


SED-4340 find-usages-of-keyword-and-plan-referenced-by-attribute-is-b…

174c638

…roken

gemini-code-assist bot reviewed Jan 8, 2026

View reviewed changes

SED-4340 PR feedbacks

5498447

david-stephan requested a review from jeromecomte January 8, 2026 13:46

jeromecomte reviewed Jan 9, 2026

View reviewed changes

david-stephan added 2 commits January 9, 2026 10:50

SED-4340 PR feedbacks

a431643

SED-4340 fixing incorrect context and predicate usage

e0c4c97

david-stephan commented Jan 9, 2026

View reviewed changes

jeromecomte reviewed Jan 9, 2026

View reviewed changes

SED-4340 find-usages-of-keyword-and-plan-referenced-by-attribute-is-b… #579

Are you sure you want to change the base?

SED-4340 find-usages-of-keyword-and-plan-referenced-by-attribute-is-b… #579

Uh oh!

Conversation

david-stephan commented Jan 8, 2026

Uh oh!

gemini-code-assist bot commented Jan 8, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist bot Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jeromecomte left a comment

Choose a reason for hiding this comment

Uh oh!

jeromecomte Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeromecomte Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

david-stephan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeromecomte Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

david-stephan Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

jeromecomte Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants