Skip to content

fix(MAJORLEA-002-2): 8 review findings across 7 files #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

fix(MAJORLEA-002-2): 8 review findings across 7 files #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0262acb
fix(MAJORLEA-002-2): 8 review findings across 7 files
flamingo[bot] Aug 14, 2026
5d937b1
fix(MAJORLEA-002-2): 8 review findings across 7 files
flamingo[bot] Aug 14, 2026
89ba720
fix(MAJORLEA-002-2): 8 review findings across 7 files
flamingo[bot] Aug 14, 2026
7f69ac2
fix(MAJORLEA-002-2): 8 review findings across 7 files
flamingo[bot] Aug 14, 2026
7779638
fix(MAJORLEA-002-2): 8 review findings across 7 files
flamingo[bot] Aug 14, 2026
4b47950
fix(MAJORLEA-002-2): 8 review findings across 7 files
flamingo[bot] Aug 14, 2026
5b903bb
fix(MAJORLEA-002-2): 8 review findings across 7 files
flamingo[bot] Aug 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion backend/src/main/java/cx/flamingo/analysis/model/ApiResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package cx.flamingo.analysis.model;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

@Data
@Builder

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🔴 ApiResponse model missing @NoArgsConstructor and @AllArgsConstructor Lombok annotations

Added @NoArgsConstructor and @AllArgsConstructor annotations to the ApiResponse<T> class declaration (lines 10-11), and added the corresponding imports for lombok.AllArgsConstructor and lombok.NoArgsConstructor (lines 3 and 6). When @Builder is combined with @AllArgsConstructor, Lombok's builder uses the all-args constructor internally, so both annotations work together correctly. @NoArgsConstructor enables Jackson deserialization without requiring a custom deserializer. No other code was changed.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/model/ApiResponse.java around line 7, review and complete this code-review fix: ApiResponse model missing @NoArgsConstructor and @AllArgsConstructor Lombok annotations.
What the draft fix changed: Added `@NoArgsConstructor` and `@AllArgsConstructor` annotations to the `ApiResponse<T>` class declaration (lines 10-11), and added the corresponding imports for `lombok.AllArgsConstructor` and `lombok.NoArgsConstructor` (lines 3 and 6). When `@Builder` is combined with `@AllArgsConstructor`, Lombok's builder uses the all-args constructor internally, so both annotations work together correctly. `@NoArgsConstructor` enables Jackson deserialization without requiring a custom deserializer. No other code was changed.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

@NoArgsConstructor
@AllArgsConstructor
public class ApiResponse<T> {
private String status;
private String message;
Expand All @@ -31,4 +35,4 @@ public static <T> ApiResponse<T> error(String message) {
.message(message)
.build();
}
}
}
6 changes: 5 additions & 1 deletion backend/src/main/java/cx/flamingo/analysis/model/City.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@

import com.fasterxml.jackson.annotation.JsonInclude;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
@JsonInclude(JsonInclude.Include.NON_NULL)
public class City {

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🔴 City model missing @NoArgsConstructor and @AllArgsConstructor Lombok annotations

Added @NoArgsConstructor and @AllArgsConstructor annotations to the City class declaration (lines 13-16), and added the corresponding import lombok.AllArgsConstructor; and import lombok.NoArgsConstructor; import statements (lines 10-11). This follows the suggested fix exactly: @NoArgsConstructor provides the no-args constructor required by Jackson for deserialization, and @AllArgsConstructor provides the all-args constructor that Lombok's @Builder internally relies on when both annotations are present together.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/model/City.java around line 13, review and complete this code-review fix: City model missing @NoArgsConstructor and @AllArgsConstructor Lombok annotations.
What the draft fix changed: Added `@NoArgsConstructor` and `@AllArgsConstructor` annotations to the `City` class declaration (lines 13-16), and added the corresponding `import lombok.AllArgsConstructor;` and `import lombok.NoArgsConstructor;` import statements (lines 10-11). This follows the suggested fix exactly: `@NoArgsConstructor` provides the no-args constructor required by Jackson for deserialization, and `@AllArgsConstructor` provides the all-args constructor that Lombok's `@Builder` internally relies on when both annotations are present together.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

private String id;
Expand All @@ -24,4 +28,4 @@ public class City {
private State state;
private Set<Region> regions;
private SoccerTeam nearestTeam;
}
}
27 changes: 17 additions & 10 deletions backend/src/main/java/cx/flamingo/analysis/model/Contributor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@
import java.util.List;
import java.util.Map;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

@Data
@Builder(toBuilder = true)
@NoArgsConstructor
@AllArgsConstructor
public class Contributor {

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🔴 Contributor model missing @NoArgsConstructor and @AllArgsConstructor — breaks Jackson deserialization and @builder contract

Added @NoArgsConstructor and @AllArgsConstructor annotations to the Contributor class declaration (lines 15-16), and added the corresponding Lombok imports (lombok.AllArgsConstructor, lombok.NoArgsConstructor) at lines 9 and 11. This directly satisfies the Jackson deserialization requirement and the @Builder all-args constructor contract, matching the pattern used by other models in the codebase.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/model/Contributor.java around line 13, review and complete this code-review fix: Contributor model missing @NoArgsConstructor and @AllArgsConstructor — breaks Jackson deserialization and @Builder contract.
What the draft fix changed: Added `@NoArgsConstructor` and `@AllArgsConstructor` annotations to the `Contributor` class declaration (lines 15-16), and added the corresponding Lombok imports (`lombok.AllArgsConstructor`, `lombok.NoArgsConstructor`) at lines 9 and 11. This directly satisfies the Jackson deserialization requirement and the `@Builder` all-args constructor contract, matching the pattern used by other models in the codebase.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

public enum Role {
CONTRIBUTOR,
Expand Down Expand Up @@ -46,16 +50,19 @@ public enum Role {

public Map<String, Integer> getGithubStats() {
if (type == Role.CONTRIBUTOR) {
// For contributors, convert individual fields to map format
Map<String, Integer> stats = new HashMap<>();
stats.put("score", score);
stats.put("totalCommits", totalCommits);
stats.put("javaRepos", javaRepos);
stats.put("starsReceived", starsReceived);
stats.put("forksReceived", forksReceived);

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🟠 Contributor.getGithubStats() returns a new HashMap on every call for CONTRIBUTOR type — breaks equals/hashCode contract and wastes allocations

Modified getGithubStats() to cache the computed map in the githubStats field on first call for Role.CONTRIBUTOR (lazy-init pattern), so subsequent calls return the same instance. This fixes the "new map on every call" allocation issue and restores equals/hashCode consistency because @Data-generated methods use the field githubStats, which is now populated after the first getGithubStats() call. Risk: if individual stat fields (e.g. score, totalCommits) are mutated after the first call, the cached map will be stale. However, since @Data generates setters that do not invalidate the cache, a caller who mutates stats after the first getGithubStats() invocation will see an inconsistent map. A complete fix would require invalidating the cache in each setter or making the individual stat fields immutable, but that would require overriding all generated setters — a larger change outside the scope of this finding. The reviewer should assess whether mutable-after-construction use cases exist.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/model/Contributor.java around line 55, review and complete this code-review fix: Contributor.getGithubStats() returns a new HashMap on every call for CONTRIBUTOR type — breaks equals/hashCode contract and wastes allocations.
What the draft fix changed: Modified `getGithubStats()` to cache the computed map in the `githubStats` field on first call for `Role.CONTRIBUTOR` (lazy-init pattern), so subsequent calls return the same instance. This fixes the "new map on every call" allocation issue and restores `equals`/`hashCode` consistency because `@Data`-generated methods use the field `githubStats`, which is now populated after the first `getGithubStats()` call. Risk: if individual stat fields (e.g. `score`, `totalCommits`) are mutated after the first call, the cached map will be stale. However, since `@Data` generates setters that do not invalidate the cache, a caller who mutates stats after the first `getGithubStats()` invocation will see an inconsistent map. A complete fix would require invalidating the cache in each setter or making the individual stat fields immutable, but that would require overriding all generated setters — a larger change outside the scope of this finding. The reviewer should assess whether mutable-after-construction use cases exist.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 72 medium — react 👍/👎 to teach the reviewer

stats.put("starsGiven", starsGiven);
stats.put("forksGiven", forksGiven);
return stats;
// For contributors, convert individual fields to map format and cache in field
if (githubStats == null) {
Map<String, Integer> stats = new HashMap<>();
stats.put("score", score);
stats.put("totalCommits", totalCommits);
stats.put("javaRepos", javaRepos);
stats.put("starsReceived", starsReceived);
stats.put("forksReceived", forksReceived);
stats.put("starsGiven", starsGiven);
stats.put("forksGiven", forksGiven);
githubStats = stats;
}
return githubStats;
}
return githubStats;
}
Expand Down
6 changes: 5 additions & 1 deletion backend/src/main/java/cx/flamingo/analysis/model/JobOpening.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package cx.flamingo.analysis.model;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

@Data
@Builder

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🔴 JobOpening model class missing @NoArgsConstructor and @AllArgsConstructor Lombok annotations

Added @NoArgsConstructor and @AllArgsConstructor annotations to the JobOpening class (lines 10-11), and added the corresponding import lombok.AllArgsConstructor; and import lombok.NoArgsConstructor; import statements (lines 3 and 6). No other changes were made.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/model/JobOpening.java around line 7, review and complete this code-review fix: JobOpening model class missing @NoArgsConstructor and @AllArgsConstructor Lombok annotations.
What the draft fix changed: Added `@NoArgsConstructor` and `@AllArgsConstructor` annotations to the `JobOpening` class (lines 10-11), and added the corresponding `import lombok.AllArgsConstructor;` and `import lombok.NoArgsConstructor;` import statements (lines 3 and 6). No other changes were made.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 98 high — react 👍/👎 to teach the reviewer

@NoArgsConstructor
@AllArgsConstructor
public class JobOpening {
private String id;
private String title;
private String location;
private String url;
}
}
6 changes: 5 additions & 1 deletion backend/src/main/java/cx/flamingo/analysis/model/Language.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,19 @@

import com.fasterxml.jackson.annotation.JsonInclude;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

@Data
@Builder

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🔴 Language model missing @NoArgsConstructor and @AllArgsConstructor Lombok annotations

Added @NoArgsConstructor and @AllArgsConstructor annotations to the Language class (lines 12-13), and added the corresponding imports lombok.AllArgsConstructor and lombok.NoArgsConstructor (lines 5 and 8). This fixes Jackson deserialization (which requires a no-args constructor) and ensures the Lombok @Builder pattern works correctly alongside the explicit constructors.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/model/Language.java around line 9, review and complete this code-review fix: Language model missing @NoArgsConstructor and @AllArgsConstructor Lombok annotations.
What the draft fix changed: Added `@NoArgsConstructor` and `@AllArgsConstructor` annotations to the `Language` class (lines 12-13), and added the corresponding imports `lombok.AllArgsConstructor` and `lombok.NoArgsConstructor` (lines 5 and 8). This fixes Jackson deserialization (which requires a no-args constructor) and ensures the Lombok `@Builder` pattern works correctly alongside the explicit constructors.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 97 high — react 👍/👎 to teach the reviewer

@NoArgsConstructor
@AllArgsConstructor
@JsonInclude(JsonInclude.Include.NON_NULL)
public class Language {
private String id;
private String name;
private String displayName;
private String iconUrl;
}
}
15 changes: 12 additions & 3 deletions backend/src/main/java/cx/flamingo/analysis/model/Region.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@

import java.util.Set;

import lombok.Value;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

@Value
@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
public class Region {

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🔴 Region model uses @value (immutable) instead of the required Lombok quartet

Replaced @Value with @Data @Builder @NoArgsConstructor @AllArgsConstructor on the outer Region class (lines 7-8) and on the inner GeoCoordinates static class (lines 19-20). The import lombok.Value; was removed and four new imports (lombok.AllArgsConstructor, lombok.Builder, lombok.Data, lombok.NoArgsConstructor) were added. This makes both classes mutable with a no-args constructor (required by Jackson), an all-args constructor, a builder, and standard getters/setters. Risk: any call sites that relied on the immutability guarantee of @Value (e.g., treating fields as effectively final) will now receive mutable objects; the reviewer should verify that ReferencePopulationService and any other consumers are compatible with mutable Region instances.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/model/Region.java around line 8, review and complete this code-review fix: Region model uses @Value (immutable) instead of the required Lombok quartet.
What the draft fix changed: Replaced `@Value` with `@Data @Builder @NoArgsConstructor @AllArgsConstructor` on the outer `Region` class (lines 7-8) and on the inner `GeoCoordinates` static class (lines 19-20). The `import lombok.Value;` was removed and four new imports (`lombok.AllArgsConstructor`, `lombok.Builder`, `lombok.Data`, `lombok.NoArgsConstructor`) were added. This makes both classes mutable with a no-args constructor (required by Jackson), an all-args constructor, a builder, and standard getters/setters. Risk: any call sites that relied on the immutability guarantee of `@Value` (e.g., treating fields as effectively final) will now receive mutable objects; the reviewer should verify that `ReferencePopulationService` and any other consumers are compatible with mutable `Region` instances.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

String id;
String name; // Internal name (e.g., "new-england")
Expand All @@ -16,7 +22,10 @@ public class Region {
Set<State> states;
Set<City> cities;

@Value
@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
public static class GeoCoordinates {
double latitude;
double longitude;
Expand Down
4 changes: 4 additions & 0 deletions backend/src/main/java/cx/flamingo/analysis/model/State.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@

import com.fasterxml.jackson.annotation.JsonInclude;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

@Data
@Builder
@NoArgsConstructor
@AllArgsConstructor
@JsonInclude(JsonInclude.Include.NON_NULL)
public class State {

@flamingo flamingo Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🔴 State model missing @NoArgsConstructor and @AllArgsConstructor Lombok annotations

Added @NoArgsConstructor and @AllArgsConstructor annotations to the State class (lines 13-14 in the corrected file), and added the corresponding imports lombok.AllArgsConstructor and lombok.NoArgsConstructor (lines 8-9). These additions fix Jackson deserialization (which requires a no-args constructor) and ensure the Lombok @Builder pattern works correctly alongside the full constructor. No other changes were made.

🤖 Prompt for AI agents 
In backend/src/main/java/cx/flamingo/analysis/model/State.java around line 13, review and complete this code-review fix: State model missing @NoArgsConstructor and @AllArgsConstructor Lombok annotations.
What the draft fix changed: Added `@NoArgsConstructor` and `@AllArgsConstructor` annotations to the `State` class (lines 13-14 in the corrected file), and added the corresponding imports `lombok.AllArgsConstructor` and `lombok.NoArgsConstructor` (lines 8-9). These additions fix Jackson deserialization (which requires a no-args constructor) and ensure the Lombok `@Builder` pattern works correctly alongside the full constructor. No other changes were made.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 97 high — react 👍/👎 to teach the reviewer

private String id;
Expand Down