SDCICD-1735: update krknai prompt and markdown-to-HTML conversion#3128
SDCICD-1735: update krknai prompt and markdown-to-HTML conversion#3128minlei98 wants to merge 1 commit intoopenshift:mainfrom
Conversation
minlei98
commented
Feb 26, 2026
- Update Krknai.yaml prompt
- Add MergeTemplates to PromptStore for loading package-local templates on top of defaults.
- Add markdown-to-HTML conversion for --report-format=html in the krkn-ai engine
- keeping the default osde2e analyzer unchanged.
|
There are test jobs defined for this repository which are not configured to run automatically. Comment |
|
@minlei98: This pull request references SDCICD-1735 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: minlei98 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
krknai-22report.html |
22b5db7 to
f5cd52d
Compare
pkg/krknai/analysisengine/engine.go
Outdated
| func markdownToHTML(content string) string { | ||
| htmlTemplate, err := krknPrompts.ReadFile(htmlTemplatePath) | ||
| if err != nil { | ||
| return content |
There was a problem hiding this comment.
should this return or log an error?
pkg/krknai/analysisengine/engine.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func markdownToHTML(content string) string { |
There was a problem hiding this comment.
We could use LLM to generate HTML using --report-format in the prompt with a condition.
But to save tokens and keep better control, we can handle the HTML conversion ourselves.
There was a problem hiding this comment.
might be worth a try if it would remove 3 dependencies
"github.com/gomarkdown/markdown"
mdhtml "github.com/gomarkdown/markdown/html"
"github.com/gomarkdown/markdown/parser"
Q: how does the prompt way consume more tokens?
There was a problem hiding this comment.
Agree, use our own HTML conversion, we could control the output better.
f5cd52d to
a66b16a
Compare
|
@minlei98: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
| ## Executive Summary (2-3 sentences) | ||
| ## Cluster Under Test (ID, name, version, type, provider, region, hypershift yes/no) | ||
| ## Test Configuration (GA params, scenarios, health check targets) | ||
| ## Run Statistics (table: totals, generations, fitness scores, types) |
There was a problem hiding this comment.
The sample html here doesn't show fitness function used. Could you add that?
| ## Test Configuration (GA params, scenarios, health check targets) | ||
| ## Run Statistics (table: totals, generations, fitness scores, types) | ||
| ## Genetic Algorithm Evolution (fitness trends, convergence, most disruptive generation) | ||
| ## Top Vulnerabilities (top 3-5 by fitness: target, impact, severity [Critical/High/Medium/Low], why it matters) |
There was a problem hiding this comment.
Is there a way to include information about type of node (eg infra) in case of "Node CPU Hog" vulnerability?
|
The generated HTML file is really nice. Super easy to read, straight to the point. Almost doesn't even read like it's generated by AI :D The only thing I could potentially ask to add would be that somewhere in that file there's a link, or a reference to how I would figure out where the must-gather is for that run. Specifically - in the generated HTML document it references a node that caused the highest fitness score. However, without knowing any more details of that node I don't know what to do with that information. I assume that it was an infra node, but without being able to know what workloads were on it I can't really do much with the information this report generated. That's my only "problem" with this - and I use the term "problem" very loosely. This is amazing! |
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> |
There was a problem hiding this comment.
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | |
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | |
| <meta http-equiv="Content-Security-Policy" content="default-src 'self'"> |
Claude: The HTML template includes inline styles but no CSP (Content Security Policy) headers. While this is a static report, if it's ever served over HTTP, it could benefit from security headers.
| p := parser.NewWithExtensions(parser.CommonExtensions | parser.AutoHeadingIDs) | ||
| renderer := mdhtml.NewRenderer(mdhtml.RendererOptions{Flags: mdhtml.CommonFlags | mdhtml.HrefTargetBlank}) | ||
| body := markdown.ToHTML([]byte(content), p, renderer) | ||
| return fmt.Sprintf(string(htmlTemplate), string(body)), nil |
There was a problem hiding this comment.
should use template parser here like this one
Claude: Fix HTML injection vulnerability - Use html/template instead of fmt.Sprintf
| TopScenariosCount int // Number of top scenarios to include (default: 10) | ||
| TopScenariosCount int // Number of top scenarios to include (default: 10) | ||
| ReportFormat string // "json" (default), "markdown", or "html" | ||
| ClusterInfo *analysisengine.ClusterInfo // Cluster metadata from CLI flags |
There was a problem hiding this comment.
Could you get ClusterInfo directly from Collector? Then you don't need that variable here at all and can pass just the collected data to template.
Thanks for the suggestion! Created card https://issues.redhat.com/browse/SDCICD-1766 |
|
|
krknai-49report.html |
