Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ public SummarizeResult summarize(SummarizeRequest request, Consumer<Map<String,
Map<String, Object> finalResult = executeAsyncJob(jobId, "summarize", request, eventHandler);

return new SummarizeResult(
(String) finalResult.getOrDefault("summary", ""),
(String) finalResult.getOrDefault("diagram", ""),
(String) finalResult.getOrDefault("diagramType", "MERMAID"));
stringValue(finalResult, "summary", ""),
stringValue(finalResult, "diagram", ""),
stringValue(finalResult, "diagramType", "MERMAID"));
}

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

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

return new AskResult((String) finalResult.getOrDefault("answer", ""));
return new AskResult(stringValue(finalResult, "answer", ""));
}

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

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

return new ReviewResult((String) finalResult.getOrDefault("review", ""));
return new ReviewResult(stringValue(finalResult, "review", ""));
}

private static String stringValue(Map<String, Object> result, String key, String defaultValue) {
Object value = result.get(key);
return value == null ? defaultValue : String.valueOf(value);
}

@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.rostilos.codecrow.queue.RedisQueueService;

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -101,6 +102,28 @@ void shouldThrowIOExceptionWhenErrorEvent() throws Exception {
.isInstanceOf(IOException.class)
.hasMessageContaining("AI service returned error: Rate limit exceeded");
}

@Test
@DisplayName("should default null summarize result fields")
void shouldDefaultNullSummarizeResultFields() throws Exception {
Map<String, Object> resultPayload = new HashMap<>();
resultPayload.put("summary", null);
resultPayload.put("diagram", null);
resultPayload.put("diagramType", null);

Map<String, Object> finalEvent = new HashMap<>();
finalEvent.put("type", "final");
finalEvent.put("result", resultPayload);

when(queueService.rightPop(anyString(), anyLong()))
.thenReturn(objectMapper.writeValueAsString(finalEvent));

AiCommandClient.SummarizeResult result = client.summarize(createSummarizeRequest(), null);

assertThat(result.summary()).isEmpty();
assertThat(result.diagram()).isEmpty();
assertThat(result.diagramType()).isEqualTo("MERMAID");
}
}

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

assertThat(result.answer()).isEqualTo("This code implements a REST API endpoint");
}

@Test
@DisplayName("should default null answer field")
void shouldDefaultNullAnswerField() throws Exception {
Map<String, Object> resultPayload = new HashMap<>();
resultPayload.put("answer", null);

Map<String, Object> finalEvent = new HashMap<>();
finalEvent.put("type", "final");
finalEvent.put("result", resultPayload);

when(queueService.rightPop(anyString(), anyLong()))
.thenReturn(objectMapper.writeValueAsString(finalEvent));

AiCommandClient.AskResult result = client.ask(createAskRequest(), null);

assertThat(result.answer()).isEmpty();
}
}

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

assertThat(result.review()).isEqualTo("## Code Review\n\nLooks good!");
}

@Test
@DisplayName("should default null review field")
void shouldDefaultNullReviewField() throws Exception {
Map<String, Object> resultPayload = new HashMap<>();
resultPayload.put("review", null);

Map<String, Object> finalEvent = new HashMap<>();
finalEvent.put("type", "final");
finalEvent.put("result", resultPayload);

when(queueService.rightPop(anyString(), anyLong()))
.thenReturn(objectMapper.writeValueAsString(finalEvent));

AiCommandClient.ReviewResult result = client.review(createReviewRequest(), null);

assertThat(result.review()).isEmpty();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,17 @@ private String generateAnswer(
);

log.info("AI answer generated successfully");
return result.answer();
String answer = result != null ? result.answer() : null;
if (!hasUsableAnswer(answer)) {
log.warn(
"AI ask result did not include usable answer content for project={}, PR={}; using fallback answer",
project.getId(),
payload.pullRequestId()
);
return generatePlaceholderAnswer(question, context, contextData);
}

return answer;
} catch (IOException e) {
log.error("Failed to generate answer via AI: {}", e.getMessage(), e);
throw new AiGenerationException("AI service failed: " + e.getMessage(), e);
Expand Down Expand Up @@ -462,7 +472,9 @@ private String generatePlaceholderAnswer(
if (!contextData.analysisInfo().isBlank()) {
answer.append(contextData.analysisInfo());
} else {
answer.append("No analysis data found for this PR yet.\n");
answer.append("I couldn't generate a detailed AI answer for this PR.\n\n");
answer.append("No analysis data was found for this PR yet. ");
answer.append("Run `/codecrow analyze` first, then retry your question.\n");
}
}
case ANALYSIS_RELATED -> {
Expand All @@ -485,7 +497,7 @@ private String generatePlaceholderAnswer(
}
default -> {
answer.append("**Answer**\n\n");
answer.append("_Full AI-powered answers are pending implementation._\n\n");
answer.append("I couldn't generate a detailed AI answer for this question.\n\n");
answer.append("Your question: \"").append(truncate(question, 200)).append("\"\n\n");
answer.append("For now, you can:\n");
answer.append("- Use `/codecrow analyze` to run PR analysis\n");
Expand All @@ -501,7 +513,11 @@ private String formatResponse(String answer, QuestionContext context) {
StringBuilder sb = new StringBuilder();
sb.append("<!-- codecrow-ask-response -->\n");
sb.append("## 💬 CodeCrow Answer\n\n");
sb.append(answer);
if (hasUsableAnswer(answer)) {
sb.append(answer);
} else {
sb.append("I couldn't generate an answer. Please try rephrasing your question.");
}

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

private boolean hasUsableAnswer(String answer) {
if (answer == null || answer.isBlank()) {
return false;
}

String normalized = answer.trim();
return !"No output generated".equalsIgnoreCase(normalized)
&& !"null".equalsIgnoreCase(normalized)
&& !"none".equalsIgnoreCase(normalized);
}

/**
* Types of questions that can be asked.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,19 @@ private SummaryResult generateSummary(

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

// Convert diagram type from string to enum
PrSummarizeCache.DiagramType resultDiagramType =
"ASCII".equalsIgnoreCase(result.diagramType())
? PrSummarizeCache.DiagramType.ASCII
: PrSummarizeCache.DiagramType.MERMAID;
PrSummarizeCache.DiagramType resultDiagramType = resolveDiagramType(
result != null ? result.diagramType() : null,
diagramType
);

if (result == null || !hasText(result.summary())) {
log.warn(
"AI summarize result did not include summary content for project={}, PR={}; using fallback summary",
project.getId(),
payload.pullRequestId()
);
return generateFallbackSummary(payload, resultDiagramType);
}

return new SummaryResult(
result.summary(),
Expand Down Expand Up @@ -390,13 +398,13 @@ private String generatePlaceholderSummary(WebhookPayload payload, PrSummarizeCac
.append("`.\n\n");

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

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

sb.append("### Recommendations\n");
sb.append("_Recommendations pending._\n\n");
sb.append("_Review the pull request manually before merging._\n\n");

return sb.toString();
}
Expand Down Expand Up @@ -443,18 +451,40 @@ private PrSummarizeCache cacheResult(
WebhookPayload payload,
SummaryResult summaryResult
) {
String summaryContent = summaryResult != null ? summaryResult.summaryContent() : null;
if (!hasText(summaryContent)) {
throw new IllegalStateException("Cannot cache empty summary content");
}

PrSummarizeCache cache = new PrSummarizeCache();
cache.setProject(project);
cache.setCommitHash(payload.commitHash());
cache.setPrNumber(Long.parseLong(payload.pullRequestId()));
cache.setSummaryContent(summaryResult.summaryContent());
cache.setSummaryContent(summaryContent);
cache.setDiagramContent(summaryResult.diagramContent());
cache.setDiagramType(summaryResult.diagramType());
cache.setExpiresAt(OffsetDateTime.now().plusHours(CACHE_TTL_HOURS));

return summarizeCacheRepository.save(cache);
}

private PrSummarizeCache.DiagramType resolveDiagramType(
String diagramType,
PrSummarizeCache.DiagramType defaultDiagramType
) {
if ("ASCII".equalsIgnoreCase(diagramType)) {
return PrSummarizeCache.DiagramType.ASCII;
}
if ("MERMAID".equalsIgnoreCase(diagramType)) {
return PrSummarizeCache.DiagramType.MERMAID;
}
return defaultDiagramType;
}

private boolean hasText(String value) {
return value != null && !value.isBlank();
}

private String formatSummaryForPosting(SummaryResult result, PrSummarizeCache.DiagramType diagramType) {
StringBuilder sb = new StringBuilder();
sb.append("<!-- codecrow-summary -->\n\n");
Expand Down
Loading