Conversation
Signed-off-by: Shekhar Saxena <shekhar.saxena@ibm.com>
Signed-off-by: bharathappali <abharath@redhat.com>
Signed-off-by: bharathappali <abharath@redhat.com>
Reviewer's GuideAdds structured RCA output fields from the LLM, wires them through the orchestrator and report model, and introduces a new two-column RCA overview UI with severity, confidence, symptoms, affected services, and supporting evidence, plus updated prompting for RCA and GC pause detection and minor placeholder text tweaks. Sequence diagram for structured RCA extraction and UI renderingsequenceDiagram
actor User
participant Browser
participant Backend
participant RcaOrchestrator
participant RootCauseAnalyst
participant GcPauseDetector
User->>Browser: Request analysis_details page
Browser->>Backend: HTTP GET /analysis_details?sessionId
Backend->>RcaOrchestrator: runAnalysisInternal(sessionId, namespace, podName)
RcaOrchestrator->>RootCauseAnalyst: analyzeRootCause(anomalyType, llmContext)
RootCauseAnalyst-->>RcaOrchestrator: rcaOutput (structured text)
RcaOrchestrator->>GcPauseDetector: detectGcPause(summarizedLogs)
GcPauseDetector-->>RcaOrchestrator: gcResult
RcaOrchestrator->>RcaOrchestrator: extractRootCause(rcaOutput)
RcaOrchestrator->>RcaOrchestrator: extractKeyEvidence(rcaOutput)
RcaOrchestrator->>RcaOrchestrator: extractSupportedLogs(rcaOutput)
RcaOrchestrator->>RcaOrchestrator: extractSeverity(rcaOutput)
RcaOrchestrator->>RcaOrchestrator: extractSupportingEvidenceBullets(rcaOutput)
RcaOrchestrator->>RcaOrchestrator: extractObservableSymptoms(rcaOutput)
RcaOrchestrator->>RcaOrchestrator: extractAffectedServices(rcaOutput)
RcaOrchestrator->>RcaOrchestrator: build finalReport map with structured fields
RcaOrchestrator-->>Backend: RcaReport (including severity, confidenceLevel, supportingEvidenceBullets, observableSymptoms, affectedServices)
Backend-->>Browser: Render analysisDetails.html with session.report
Browser-->>User: Show two column RCA overview (main issue and supporting evidence)
Updated class diagram for RCA orchestration and report modelclassDiagram
class RcaOrchestrator {
-Logger LOG
-ObjectMapper mapper
+void runAnalysisInternal(String sessionId, String namespace, String podName)
-String extractRootCause(String rcaOutput)
-List~String~ extractSupportedLogs(String rcaOutput)
-String extractKeyEvidence(String rcaOutput)
-String parseGcAnomalyType(String raw)
-String extractSeverity(String rcaOutput)
-List~String~ extractSupportingEvidenceBullets(String rcaOutput)
-List~String~ extractObservableSymptoms(String rcaOutput)
-List~String~ extractAffectedServices(String rcaOutput)
}
class RootCauseAnalyst {
<<interface>>
+String analyzeRootCause(String anomalyType, String llmContext)
}
class GcPauseDetector {
<<interface>>
+String detectGcPause(String summarizedLogs)
}
class RcaReport {
+String title
+String issue
+String highLevelIssue
+String subLevelIssue
+String anomalyType
+List~String~ validationChecks
+List~AssertionItem~ assertions
+FinalDecision finalDecision
+String severity
+String confidenceLevel
+List~String~ supportingEvidenceBullets
+List~String~ observableSymptoms
+List~String~ affectedServices
}
class AssertionItem
class FinalDecision
RcaOrchestrator --> RootCauseAnalyst : uses
RcaOrchestrator --> GcPauseDetector : uses
RcaOrchestrator --> RcaReport : populates
RcaReport "*" --> "*" AssertionItem : contains
RcaReport --> FinalDecision : has
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In RcaOrchestrator, the new
extract*helpers all parse sections fromrcaOutputwith very similar regex boundaries; consider centralizing shared parsing logic or at least the section delimiters so that future format changes to the LLM output require updates in only one place and reduce the risk of subtle mismatches. - The new RCA HTML layout drops the previous
highLevelIssueandsubLevelIssuetabbed views entirely; if those fields are still populated server-side, consider preserving them somewhere in the new UI (e.g., as expandable sections) to avoid losing potentially valuable context for users. - The additional logging in
runAnalysisInternaluses string concatenation forLOG.info; switching to parameterized logging (e.g.,LOG.info("Extracted {} supported logs from RCA Analyst", supportedLogs.size());) will avoid unnecessary string construction when the log level is lower and keep log statements consistent with typical logging best practices.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In RcaOrchestrator, the new `extract*` helpers all parse sections from `rcaOutput` with very similar regex boundaries; consider centralizing shared parsing logic or at least the section delimiters so that future format changes to the LLM output require updates in only one place and reduce the risk of subtle mismatches.
- The new RCA HTML layout drops the previous `highLevelIssue` and `subLevelIssue` tabbed views entirely; if those fields are still populated server-side, consider preserving them somewhere in the new UI (e.g., as expandable sections) to avoid losing potentially valuable context for users.
- The additional logging in `runAnalysisInternal` uses string concatenation for `LOG.info`; switching to parameterized logging (e.g., `LOG.info("Extracted {} supported logs from RCA Analyst", supportedLogs.size());`) will avoid unnecessary string construction when the log level is lower and keep log statements consistent with typical logging best practices.
## Individual Comments
### Comment 1
<location path="src/main/java/com/causa/rca/service/RcaOrchestrator.java" line_range="526-527" />
<code_context>
+ }
+
+ // Extract SUPPORTED LOGS section
+ Pattern pattern = Pattern.compile(
+ "(?i)SUPPORTED[_ ]LOGS\\s*:\\s*(.*?)(?=\\n\\n|$)",
+ Pattern.DOTALL);
+
</code_context>
<issue_to_address>
**issue (bug_risk):** SUPPORTED_LOGS regex can accidentally consume following sections and treat them as logs.
Because the pattern only stops at a blank line or end-of-string, a section header on the very next line (e.g. `KEY_EVIDENCE:`) will be captured into `logsSection` and may be treated as a supported log if it passes the length filter. Please tighten the lookahead to stop at section headers as well, e.g. `(?=\n[A-Z_ ]+\s*:|\n\n|$)`, or at least include known headers like `KEY[_ ]EVIDENCE` in the lookahead.
</issue_to_address>
### Comment 2
<location path="src/main/java/com/causa/rca/service/RcaOrchestrator.java" line_range="532-541" />
<code_context>
+
+ Matcher matcher = pattern.matcher(rcaOutput);
+
+ if (matcher.find()) {
+ String logsSection = matcher.group(1).trim();
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Severity extraction is very strict and may silently fall back to Medium for slightly noisy outputs.
The parser only accepts exact `High|Medium|Low` matches, so outputs like `High - user impact` or `High.` will fail the regex and silently fall back to `"Medium"`. To reduce misclassification risk, normalize the extracted value first (e.g., take the first word and/or strip non-letter characters) before validating against the allowed severities.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| Pattern pattern = Pattern.compile( | ||
| "(?i)SUPPORTED[_ ]LOGS\\s*:\\s*(.*?)(?=\\n\\n|$)", |
There was a problem hiding this comment.
issue (bug_risk): SUPPORTED_LOGS regex can accidentally consume following sections and treat them as logs.
Because the pattern only stops at a blank line or end-of-string, a section header on the very next line (e.g. KEY_EVIDENCE:) will be captured into logsSection and may be treated as a supported log if it passes the length filter. Please tighten the lookahead to stop at section headers as well, e.g. (?=\n[A-Z_ ]+\s*:|\n\n|$), or at least include known headers like KEY[_ ]EVIDENCE in the lookahead.
| if (matcher.find()) { | ||
| String logsSection = matcher.group(1).trim(); | ||
|
|
||
| // Skip if it says "no direct supported logs present" | ||
| if (logsSection.toLowerCase().contains("no direct supported logs present")) { | ||
| LOG.info("RCA Analyst indicated no direct supported logs present"); | ||
| return logs; | ||
| } | ||
|
|
||
| // Split by newlines and filter out empty lines |
There was a problem hiding this comment.
issue (bug_risk): Severity extraction is very strict and may silently fall back to Medium for slightly noisy outputs.
The parser only accepts exact High|Medium|Low matches, so outputs like High - user impact or High. will fail the regex and silently fall back to "Medium". To reduce misclassification risk, normalize the extracted value first (e.g., take the first word and/or strip non-letter characters) before validating against the allowed severities.
This PR is built on top of PR #33
Please review this PR after merging the #33
Summary by Sourcery
Introduce a richer, structured root cause analysis experience with new LLM prompts, parsing logic, and UI layout to surface severity, confidence, evidence, symptoms, and affected services.
New Features:
Enhancements: