Conversation
…ions during registration
- Introduced ProjectRulesConfig to manage custom review rules for projects. - Added RuleType enum to define rule types (ENFORCE, SUPPRESS). - Implemented UpdateProjectRulesRequest DTO for updating rules. - Enhanced ProjectDTO to include project rules configuration. - Updated ProjectService to handle retrieval and updating of project rules. - Integrated project rules into AI client services for Bitbucket, GitHub, and GitLab. - Added endpoints in ProjectController for fetching and updating project rules. - Updated Python inference orchestrator to format and utilize project rules in review stages. - Added configuration property for maximum custom rules per project. - Updated application properties sample to include project rules configuration.
- Updated GitHubAiClientService and GitLabAiClientService to include handling of deleted files during analysis requests. - Modified AiAnalysisRequest DTO to add a new field for deleted files. - Enhanced context formatting in RAG to filter out chunks from deleted files, ensuring stale references are not flagged. - Updated prompt builder to include context about deleted files in the generated prompts for better LLM understanding. - Improved logging to track the number of skipped chunks from deleted files during analysis.
feat: add custom project review rules configuration
…ist-in-analysis-context Bugfix/ca 17 deleted files persist in analysis context
|
/codecrow analyze |
|
Comment commands are not enabled for this project Check the job logs in CodeCrow for detailed error information. |
|
| Status | PASS WITH WARNINGS |
| Risk Level | HIGH |
| Review Coverage | 29 files analyzed in depth |
| Confidence | HIGH |
Executive Summary
This PR introduces version 1.4.1 rc, focusing on updates to project rule configurations, diff parsing utilities, and authentication logic across the Java and Python ecosystems. While the functional scope is comprehensive, the review identified significant architectural fragmentation regarding diff parsing and duplicated authorization logic for administrative provisioning. Additionally, brittle serialization and missing validation in the project rules pipeline pose a risk to downstream Python orchestrator stability.
Recommendation
Decision: PASS WITH WARNINGS
The PR is architecturally sound in intent but requires immediate remediation of duplicated "first-user-admin" logic and the implementation of robust JSON serialization for project rules to prevent runtime failures. Coordination between the Java and Python diff parsing strategies is strongly recommended to ensure consistent VCS analysis.
Issues Overview
| Severity | Count | |
|---|---|---|
| 🔴 High | 2 | Critical issues requiring immediate attention |
| 🟡 Medium | 5 | Issues that should be addressed |
| 🔵 Low | 1 | Minor issues and improvements |
Analysis completed on 2026-02-19 12:55:59 | View Full Report | Pull Request
📋 Detailed Issues (8)
🔴 High Severity Issues
Id on Platform: 3333
Category: 🏗️ Architecture
File: .../controller/AuthController.java:189
Issue: Duplicated logic for 'first user becomes admin' found in both AuthController.java and GoogleOAuthService.java. As seen in the codebase context, these services handle authentication differently but share identical business logic for account promotion.
💡 Suggested Fix
Encapsulate the 'first user' logic into a shared service (e.g., a UserService or AccountPromotionService) to ensure consistency and a single point of synchronization.
No suggested fix providedId on Platform: 3336
Category: 🏗️ Architecture
File: .../util/DiffParser.java
Issue: The new extractDeletedFiles method reimplements logic already present in the Python ecosystem's python-ecosystem/inference-orchestrator/utils/diff_parser.py (method get_changed_file_paths excluding 'deleted' vs this one including only 'deleted'). While cross-language duplication is sometimes necessary for local processing, having divergent diff parsing logic across the Java and Python components increases maintenance risk when new diff formats are introduced.
💡 Suggested Fix
Ensure that both the Java and Python diff parsing utilities follow the same extraction rules or consider centralizing diff parsing if possible. At minimum, ensure the Java implementation handles all change types consistently with the Python counterpart.
No suggested fix provided🟡 Medium Severity Issues
Id on Platform: 3329
Category: 🧹 Code Quality
File: .../config/ProjectRulesConfig.java
Issue: The manual JSON escaping logic in 'escapeJson' is incomplete. It handles basic characters but misses control characters (0x00-0x1F) which are required for valid JSON according to RFC 8259.
💡 Suggested Fix
Use a robust JSON library like Jackson (already present in the project) for serialization, or expand the escape logic to cover all control characters.
--- a/java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/model/project/config/ProjectRulesConfig.java
+++ b/java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/model/project/config/ProjectRulesConfig.java
@@ -118,12 +118,18 @@
private static String escapeJson(String s) {
if (s == null) return "";
- return s.replace("\\", "\\\\")
- .replace("\"", "\\\"")
- .replace("\n", "\\n")
- .replace("\r", "\\r")
- .replace("\t", "\\t");
+ StringBuilder sb = new StringBuilder();
+ for (int i = 0; i < s.length(); i++) {
+ char ch = s.charAt(i);
+ switch (ch) {
+ case '"': sb.append("\\\""); break;
+ case '\\': sb.append("\\\\"); break;
+ case '\b': sb.append("\\b"); break;
+ case '\f': sb.append("\\f"); break;
+ case '\n': sb.append("\\n"); break;
+ case '\r': sb.append("\\r"); break;
+ case '\t': sb.append("\\t"); break;
+ default:
+ if (ch <= '\u001F') {
+ String ss = Integer.toHexString(ch);
+ sb.append("\\u").append("0".repeat(4 - ss.length())).append(ss);
+ } else {
+ sb.append(ch);
+ }
+ }
+ }
+ return sb.toString();
}Id on Platform: 3330
Category: 🐛 Bug Risk
File: .../service/ProjectService.java
Issue: When currentConfig is null, a new ProjectConfig is instantiated but it is never associated with the project entity until after the setProjectRules call. If other parts of the ProjectConfig are required by the persistence layer or JPA lifecycle listeners, this could cause partial state issues.
💡 Suggested Fix
Ensure the new configuration object is associated with the project entity immediately upon instantiation.
--- a/java-ecosystem/services/web-server/src/main/java/org/rostilos/codecrow/webserver/project/service/ProjectService.java
+++ b/java-ecosystem/services/web-server/src/main/java/org/rostilos/codecrow/webserver/project/service/ProjectService.java
@@ -798,2 +798,3 @@
if (currentConfig == null) {
currentConfig = new ProjectConfig();
+ project.setConfiguration(currentConfig);
}Id on Platform: 3332
Category: ✨ Best Practices
File: .../service/GoogleOAuthService.java:109
Issue: Synchronizing on 'UserRepository.class' is a heavy global lock and may not work in a clustered/distributed environment (multiple JVMs).
💡 Suggested Fix
For a production-ready fix in a distributed environment, use a database-level lock (e.g., SELECT ... FOR UPDATE) or a unique constraint on an 'is_admin' column if only one admin is allowed, or use a distributed lock (Redis/Zookeeper). If sticking to a single-node setup, this is acceptable but suboptimal.
No suggested fix providedId on Platform: 3334
Category: 🔒 Security
File: .../controller/ProjectController.java:565
Issue: The 'getProjectRules' endpoint lacks security annotations like @IsWorkspaceMember or @HasOwnerOrAdminRights. While the logic fetches by workspaceSlug, it might allow unauthorized users to view internal project review instructions.
💡 Suggested Fix
Add @IsWorkspaceMember to ensure only members of the workspace can view the review rules.
--- a/java-ecosystem/services/web-server/src/main/java/org/rostilos/codecrow/webserver/project/controller/ProjectController.java
+++ b/java-ecosystem/services/web-server/src/main/java/org/rostilos/codecrow/webserver/project/controller/ProjectController.java
@@ -564,6 +564,7 @@
* Returns the custom review rules configuration for the project.
*/
@GetMapping("/{projectNamespace}/project-rules")
+ @IsWorkspaceMember
public ResponseEntity<ProjectRulesConfig> getProjectRules(Id on Platform: 3335
Category: 🧹 Code Quality
File: .../request/UpdateProjectRulesRequest.java:31
Issue: The CustomRuleRequest record fields lack validation constraints like @notblank or @SiZe, even though the Javadoc mentions requirements (max 200/2000 chars). Since the controller uses @Valid, these constraints should be present to actually enforce the limits.
💡 Suggested Fix
Add Jakarta Validation annotations to the CustomRuleRequest fields.
No suggested fix provided🔵 Low Severity Issues
Id on Platform: 3331
Category: ⚡ Performance
File: .../orchestrator/stages.py
Issue: Nested loop with fnmatch may become slow if a project defines a very large number of rules and a batch contains many files. For most projects (max 20 rules), this is negligible, but computationally inefficient.
💡 Suggested Fix
Consider pre-compiling regex versions of the patterns if the number of rules increases, though for the current limit of 20, the current logic is acceptable.
No suggested fix providedFiles Affected
- .../orchestrator/stages.py: 1 issue
- .../request/UpdateProjectRulesRequest.java: 1 issue
- .../service/GoogleOAuthService.java: 1 issue
- .../service/ProjectService.java: 1 issue
- .../controller/AuthController.java: 1 issue
- .../controller/ProjectController.java: 1 issue
- .../config/ProjectRulesConfig.java: 1 issue
- .../util/DiffParser.java: 1 issue
1 participant
This pull request introduces enhancements to the AI analysis request data model, adds support for custom project review rules, and improves diff parsing utilities. The most important changes are grouped below by theme.
Custom Project Review Rules Support
application.properties.sampleto set the maximum number of custom review rules per project (codecrow.project.rules.max-count).projectRulesfield (JSON-serializable list) toAiAnalysisRequestImpl, its builder, and associated getter/setter methods, enabling custom project review rules to be included in AI analysis requests. [1] [2] [3] [4] [5]Diff Parsing and File Change Tracking
DiffParserutility with a newextractDeletedFilesmethod to identify deleted files from the diff, allowing the AI analysis request to track both changed and deleted files.AiAnalysisRequestandAiAnalysisRequestImplto include adeletedFilesfield, its builder method, and getter, so deleted files are explicitly tracked and available for analysis. [1] [2] [3] [4] [5] [6]Code Quality and Formatting Improvements
DiffParserandAiAnalysisRequestImplfor better readability and maintainability. [1] [2] [3] [4] [5] [6] [7] [8] [9]Frontend Update
frontendsubproject commit reference to the latest version, ensuring compatibility with backend changes.