Skip to content

Commit 7a2edbc

Browse files
rostilosrostilos
authored
Merge pull request #182 from rostilos/1.5.7-rc
feat: Enhance AI command processing with fallback handling and null c…
2 parents 4e5424e + 6bcbb15 commit 7a2edbc

6 files changed

Lines changed: 422 additions & 18 deletions

File tree

‎java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/aiclient/AiCommandClient.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ public SummarizeResult summarize(SummarizeRequest request, Consumer<Map<String,
4141
Map<String, Object> finalResult = executeAsyncJob(jobId, "summarize", request, eventHandler);
4242

4343
return new SummarizeResult(
44-
(String) finalResult.getOrDefault("summary", ""),
45-
(String) finalResult.getOrDefault("diagram", ""),
46-
(String) finalResult.getOrDefault("diagramType", "MERMAID"));
44+
stringValue(finalResult, "summary", ""),
45+
stringValue(finalResult, "diagram", ""),
46+
stringValue(finalResult, "diagramType", "MERMAID"));
4747
}
4848

4949
/**
@@ -56,7 +56,7 @@ public AskResult ask(AskRequest request, Consumer<Map<String, Object>> eventHand
5656

5757
Map<String, Object> finalResult = executeAsyncJob(jobId, "ask", request, eventHandler);
5858

59-
return new AskResult((String) finalResult.getOrDefault("answer", ""));
59+
return new AskResult(stringValue(finalResult, "answer", ""));
6060
}
6161

6262
/**
@@ -69,7 +69,12 @@ public ReviewResult review(ReviewRequest request, Consumer<Map<String, Object>>
6969

7070
Map<String, Object> finalResult = executeAsyncJob(jobId, "review", request, eventHandler);
7171

72-
return new ReviewResult((String) finalResult.getOrDefault("review", ""));
72+
return new ReviewResult(stringValue(finalResult, "review", ""));
73+
}
74+
75+
private static String stringValue(Map<String, Object> result, String key, String defaultValue) {
76+
Object value = result.get(key);
77+
return value == null ? defaultValue : String.valueOf(value);
7378
}
7479

7580
@SuppressWarnings("unchecked")

‎java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/aiclient/AiCommandClientTest.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.rostilos.codecrow.queue.RedisQueueService;
1313

1414
import java.io.IOException;
15+
import java.util.HashMap;
1516
import java.util.Map;
1617

1718
import static org.assertj.core.api.Assertions.assertThat;
@@ -101,6 +102,28 @@ void shouldThrowIOExceptionWhenErrorEvent() throws Exception {
101102
.isInstanceOf(IOException.class)
102103
.hasMessageContaining("AI service returned error: Rate limit exceeded");
103104
}
105+
106+
@Test
107+
@DisplayName("should default null summarize result fields")
108+
void shouldDefaultNullSummarizeResultFields() throws Exception {
109+
Map<String, Object> resultPayload = new HashMap<>();
110+
resultPayload.put("summary", null);
111+
resultPayload.put("diagram", null);
112+
resultPayload.put("diagramType", null);
113+
114+
Map<String, Object> finalEvent = new HashMap<>();
115+
finalEvent.put("type", "final");
116+
finalEvent.put("result", resultPayload);
117+
118+
when(queueService.rightPop(anyString(), anyLong()))
119+
.thenReturn(objectMapper.writeValueAsString(finalEvent));
120+
121+
AiCommandClient.SummarizeResult result = client.summarize(createSummarizeRequest(), null);
122+
123+
assertThat(result.summary()).isEmpty();
124+
assertThat(result.diagram()).isEmpty();
125+
assertThat(result.diagramType()).isEqualTo("MERMAID");
126+
}
104127
}
105128

106129
@Nested
@@ -121,6 +144,24 @@ void shouldSuccessfullyAnswerQuestion() throws Exception {
121144

122145
assertThat(result.answer()).isEqualTo("This code implements a REST API endpoint");
123146
}
147+
148+
@Test
149+
@DisplayName("should default null answer field")
150+
void shouldDefaultNullAnswerField() throws Exception {
151+
Map<String, Object> resultPayload = new HashMap<>();
152+
resultPayload.put("answer", null);
153+
154+
Map<String, Object> finalEvent = new HashMap<>();
155+
finalEvent.put("type", "final");
156+
finalEvent.put("result", resultPayload);
157+
158+
when(queueService.rightPop(anyString(), anyLong()))
159+
.thenReturn(objectMapper.writeValueAsString(finalEvent));
160+
161+
AiCommandClient.AskResult result = client.ask(createAskRequest(), null);
162+
163+
assertThat(result.answer()).isEmpty();
164+
}
124165
}
125166

126167
@Nested
@@ -141,5 +182,23 @@ void shouldSuccessfullyReviewCode() throws Exception {
141182

142183
assertThat(result.review()).isEqualTo("## Code Review\n\nLooks good!");
143184
}
185+
186+
@Test
187+
@DisplayName("should default null review field")
188+
void shouldDefaultNullReviewField() throws Exception {
189+
Map<String, Object> resultPayload = new HashMap<>();
190+
resultPayload.put("review", null);
191+
192+
Map<String, Object> finalEvent = new HashMap<>();
193+
finalEvent.put("type", "final");
194+
finalEvent.put("result", resultPayload);
195+
196+
when(queueService.rightPop(anyString(), anyLong()))
197+
.thenReturn(objectMapper.writeValueAsString(finalEvent));
198+
199+
AiCommandClient.ReviewResult result = client.review(createReviewRequest(), null);
200+
201+
assertThat(result.review()).isEmpty();
202+
}
144203
}
145204
}

‎java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/generic/processor/command/AskCommandProcessor.java‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,17 @@ private String generateAnswer(
297297
);
298298

299299
log.info("AI answer generated successfully");
300-
return result.answer();
300+
String answer = result != null ? result.answer() : null;
301+
if (!hasUsableAnswer(answer)) {
302+
log.warn(
303+
"AI ask result did not include usable answer content for project={}, PR={}; using fallback answer",
304+
project.getId(),
305+
payload.pullRequestId()
306+
);
307+
return generatePlaceholderAnswer(question, context, contextData);
308+
}
309+
310+
return answer;
301311
} catch (IOException e) {
302312
log.error("Failed to generate answer via AI: {}", e.getMessage(), e);
303313
throw new AiGenerationException("AI service failed: " + e.getMessage(), e);
@@ -462,7 +472,9 @@ private String generatePlaceholderAnswer(
462472
if (!contextData.analysisInfo().isBlank()) {
463473
answer.append(contextData.analysisInfo());
464474
} else {
465-
answer.append("No analysis data found for this PR yet.\n");
475+
answer.append("I couldn't generate a detailed AI answer for this PR.\n\n");
476+
answer.append("No analysis data was found for this PR yet. ");
477+
answer.append("Run `/codecrow analyze` first, then retry your question.\n");
466478
}
467479
}
468480
case ANALYSIS_RELATED -> {
@@ -485,7 +497,7 @@ private String generatePlaceholderAnswer(
485497
}
486498
default -> {
487499
answer.append("**Answer**\n\n");
488-
answer.append("_Full AI-powered answers are pending implementation._\n\n");
500+
answer.append("I couldn't generate a detailed AI answer for this question.\n\n");
489501
answer.append("Your question: \"").append(truncate(question, 200)).append("\"\n\n");
490502
answer.append("For now, you can:\n");
491503
answer.append("- Use `/codecrow analyze` to run PR analysis\n");
@@ -501,7 +513,11 @@ private String formatResponse(String answer, QuestionContext context) {
501513
StringBuilder sb = new StringBuilder();
502514
sb.append("<!-- codecrow-ask-response -->\n");
503515
sb.append("## 💬 CodeCrow Answer\n\n");
504-
sb.append(answer);
516+
if (hasUsableAnswer(answer)) {
517+
sb.append(answer);
518+
} else {
519+
sb.append("I couldn't generate an answer. Please try rephrasing your question.");
520+
}
505521

506522
String content = sb.toString();
507523
if (content.length() > MAX_RESPONSE_LENGTH) {
@@ -517,6 +533,17 @@ private String truncate(String text, int maxLength) {
517533
return text.substring(0, maxLength) + "...";
518534
}
519535

536+
private boolean hasUsableAnswer(String answer) {
537+
if (answer == null || answer.isBlank()) {
538+
return false;
539+
}
540+
541+
String normalized = answer.trim();
542+
return !"No output generated".equalsIgnoreCase(normalized)
543+
&& !"null".equalsIgnoreCase(normalized)
544+
&& !"none".equalsIgnoreCase(normalized);
545+
}
546+
520547
/**
521548
* Types of questions that can be asked.
522549
*/

‎java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/generic/processor/command/SummarizeCommandProcessor.java‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -217,11 +217,19 @@ private SummaryResult generateSummary(
217217

218218
log.info("AI summarization completed successfully");
219219

220-
// Convert diagram type from string to enum
221-
PrSummarizeCache.DiagramType resultDiagramType =
222-
"ASCII".equalsIgnoreCase(result.diagramType())
223-
? PrSummarizeCache.DiagramType.ASCII
224-
: PrSummarizeCache.DiagramType.MERMAID;
220+
PrSummarizeCache.DiagramType resultDiagramType = resolveDiagramType(
221+
result != null ? result.diagramType() : null,
222+
diagramType
223+
);
224+
225+
if (result == null || !hasText(result.summary())) {
226+
log.warn(
227+
"AI summarize result did not include summary content for project={}, PR={}; using fallback summary",
228+
project.getId(),
229+
payload.pullRequestId()
230+
);
231+
return generateFallbackSummary(payload, resultDiagramType);
232+
}
225233

226234
return new SummaryResult(
227235
result.summary(),
@@ -390,13 +398,13 @@ private String generatePlaceholderSummary(WebhookPayload payload, PrSummarizeCac
390398
.append("`.\n\n");
391399

392400
sb.append("### Key Changes\n");
393-
sb.append("_Summary generation via AI is pending implementation._\n\n");
401+
sb.append("_CodeCrow could not generate a detailed AI summary from the AI response._\n\n");
394402

395403
sb.append("### Impact Analysis\n");
396-
sb.append("_Analysis pending._\n\n");
404+
sb.append("_Check the job logs for the underlying AI response details._\n\n");
397405

398406
sb.append("### Recommendations\n");
399-
sb.append("_Recommendations pending._\n\n");
407+
sb.append("_Review the pull request manually before merging._\n\n");
400408

401409
return sb.toString();
402410
}
@@ -443,18 +451,40 @@ private PrSummarizeCache cacheResult(
443451
WebhookPayload payload,
444452
SummaryResult summaryResult
445453
) {
454+
String summaryContent = summaryResult != null ? summaryResult.summaryContent() : null;
455+
if (!hasText(summaryContent)) {
456+
throw new IllegalStateException("Cannot cache empty summary content");
457+
}
458+
446459
PrSummarizeCache cache = new PrSummarizeCache();
447460
cache.setProject(project);
448461
cache.setCommitHash(payload.commitHash());
449462
cache.setPrNumber(Long.parseLong(payload.pullRequestId()));
450-
cache.setSummaryContent(summaryResult.summaryContent());
463+
cache.setSummaryContent(summaryContent);
451464
cache.setDiagramContent(summaryResult.diagramContent());
452465
cache.setDiagramType(summaryResult.diagramType());
453466
cache.setExpiresAt(OffsetDateTime.now().plusHours(CACHE_TTL_HOURS));
454467

455468
return summarizeCacheRepository.save(cache);
456469
}
457470

471+
private PrSummarizeCache.DiagramType resolveDiagramType(
472+
String diagramType,
473+
PrSummarizeCache.DiagramType defaultDiagramType
474+
) {
475+
if ("ASCII".equalsIgnoreCase(diagramType)) {
476+
return PrSummarizeCache.DiagramType.ASCII;
477+
}
478+
if ("MERMAID".equalsIgnoreCase(diagramType)) {
479+
return PrSummarizeCache.DiagramType.MERMAID;
480+
}
481+
return defaultDiagramType;
482+
}
483+
484+
private boolean hasText(String value) {
485+
return value != null && !value.isBlank();
486+
}
487+
458488
private String formatSummaryForPosting(SummaryResult result, PrSummarizeCache.DiagramType diagramType) {
459489
StringBuilder sb = new StringBuilder();
460490
sb.append("<!-- codecrow-summary -->\n\n");

0 commit comments

Comments
 (0)