Skip to content

SDCICD-1735: update krknai prompt and markdown-to-HTML conversion#3128

Open
minlei98 wants to merge 1 commit intoopenshift:mainfrom
minlei98:SDCICD-1735
Open

SDCICD-1735: update krknai prompt and markdown-to-HTML conversion#3128
minlei98 wants to merge 1 commit intoopenshift:mainfrom
minlei98:SDCICD-1735

Conversation

@minlei98
Copy link
Contributor

  1. Update Krknai.yaml prompt
  2. Add MergeTemplates to PromptStore for loading package-local templates on top of defaults.
  3. Add markdown-to-HTML conversion for --report-format=html in the krkn-ai engine
  4. keeping the default osde2e analyzer unchanged.

@openshift-ci-robot
Copy link

There are test jobs defined for this repository which are not configured to run automatically. Comment /test ? to see a list of all defined jobs. Review these jobs and use /test <job> to manually trigger jobs most likely to be impacted by the proposed changes.Comment /pipeline required to trigger all required & necessary jobs.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 26, 2026
@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 26, 2026

@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.

Details

In response to this:

  1. Update Krknai.yaml prompt
  2. Add MergeTemplates to PromptStore for loading package-local templates on top of defaults.
  3. Add markdown-to-HTML conversion for --report-format=html in the krkn-ai engine
  4. keeping the default osde2e analyzer unchanged.

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 26, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: minlei98
Once this PR has been reviewed and has the lgtm label, please assign yiqinzhang for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@minlei98
Copy link
Contributor Author

krknai-22report.html
Report with cluster info

@minlei98 minlei98 force-pushed the SDCICD-1735 branch 2 times, most recently from 22b5db7 to f5cd52d Compare February 26, 2026 21:04
func markdownToHTML(content string) string {
htmlTemplate, err := krknPrompts.ReadFile(htmlTemplatePath)
if err != nil {
return content
Copy link
Contributor

@ritmun ritmun Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this return or log an error?

return nil
}

func markdownToHTML(content string) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@ritmun ritmun Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, use our own HTML conversion, we could control the output better.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 27, 2026

@minlei98: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 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)
Copy link
Contributor

@ritmun ritmun Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Contributor

@ritmun ritmun Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to include information about type of node (eg infra) in case of "Node CPU Hog" vulnerability?

@iamkirkbater
Copy link
Contributor

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">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

func (a *KrknAIAggregator) Collect(ctx context.Context, resultsDir string) (*KrknAIData, error) {

@ritmun
Copy link
Contributor

ritmun commented Mar 2, 2026

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

Thanks for the suggestion! Created card https://issues.redhat.com/browse/SDCICD-1766

@ritmun
Copy link
Contributor

ritmun commented Mar 2, 2026

krknai-22report.html Report with cluster info

  1. Can we add environment and remove cluster name?
  2. Cluster Type can combine - cloud provider (AWS/GCP), ocp type (ROSA/OSD) and hypershift (if hcp)

@minlei98
Copy link
Contributor Author

minlei98 commented Mar 2, 2026

krknai-49report.html
Here is the latest report with last week's test result.

@ritmun
Copy link
Contributor

ritmun commented Mar 2, 2026

Can we increase the font size for this highlighted code style to normal size?
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants