Skip to content

add feature docs for symptoms#3607

Closed
sosiouxme wants to merge 1 commit into
openshift:mainfrom
sosiouxme:20260605-retroactive-symptoms
Closed

add feature docs for symptoms#3607
sosiouxme wants to merge 1 commit into
openshift:mainfrom
sosiouxme:20260605-retroactive-symptoms

Conversation

@sosiouxme

@sosiouxme sosiouxme commented Jun 10, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Documentation

    • Added a comprehensive Symptoms & Labels feature doc covering concepts, end-to-end data flow, APIs, storage, and current status.
    • Introduced a rule requiring feature-affecting changes to include corresponding updates to feature docs; contributor guidance updated.
  • Chores

    • Added reviewer guidance and a pre-merge “Feature Documentation” warning to surface missing feature-doc updates.
    • Updated Playwright MCP server command to use the latest package.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 10, 2026
@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sosiouxme

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

The pull request process is described 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

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Adds documentation governance rules and review automation prompts, regenerates related docs/config, and adds a comprehensive “Symptoms and Labels” feature document covering concepts, data flow, code locations, APIs, storage, and roadmap items.

Changes

Feature Documentation for Symptoms and Labels

Layer / File(s) Summary
Documentation governance and review automation
.apm/instructions/docs.instructions.md, .coderabbit.yaml, .mcp.json, AGENTS.md, CLAUDE.md
APM instructions and review configuration now require or prompt updates to docs/features/ when code changes affect documented features; added path-specific doc prompts and a pre-merge “Feature Documentation” warning; updated Playwright MCP invocation and regenerated docs headers.
Symptoms and Labels feature documentation
docs/features/job-analysis-symptoms.md
New feature doc describing goals, core concepts (labels/symptoms/matching), end-to-end pipeline (definition → detection → storage → ingestion), triage and UI surfaces, key code locations, REST API endpoints under /api/jobs/, storage locations (Postgres, BigQuery, GCS), and active/planned work items.

🎯 1 (Trivial) | ⏱️ ~3 minutes


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
No-Sensitive-Data-In-Logs ❌ Error The PR adds hardcoded default credentials in multiple documentation and instruction files (postgresql://postgres:password@...), which exposes the database password in plaintext throughout develop... Replace hardcoded credentials with environment variable references (e.g., use $SIPPY_DATABASE_DSN instead of literal passwords) and document how to set these in .env.example or setup instructions.
Go Error Handling ⚠️ Warning Multiple Go error handling violations found: (1) Ignored error in cmd/sippy/load.go:397 with opCtx, _ := bqcachedclient.OpCtxForCronEnv(), (2) Panic calls in main commands outside init() (cmd/sip... Remove ignored error _ assignment or check error; replace panics with proper error returns in command handlers; wrap errors with %w in fmt.Errorf calls for error chain preservation.
✅ Passed checks (19 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sql Injection Prevention ✅ Passed PR code uses parameterized SQL queries with ? placeholders and GORM ORM, avoiding direct string concatenation with user input. No SQL injection vulnerabilities detected.
Excessive Css In React Should Use Styles ✅ Passed This PR adds feature documentation and updates configuration files only. No React component code with inline CSS was modified, making the check inapplicable.
Test Coverage For New Features ✅ Passed PR adds documentation (feature docs, instruction rules) and configuration files (.coderabbit.yaml, .mcp.json) with no new Go functions, methods, or functional code requiring test coverage. Changes...
Single Responsibility And Clear Naming ✅ Passed PR #3607 contains only configuration and documentation changes (no code modifications). The check about "Single Responsibility and Clear Naming" applies specifically to packages, structs, methods,...
Feature Documentation ✅ Passed PR includes comprehensive feature documentation (docs/features/job-analysis-symptoms.md) covering purpose, data flow, API endpoints, code locations, and core concepts for all 11 modified symptom-re...
Stable And Deterministic Test Names ✅ Passed PR contains only documentation and configuration file changes; no Ginkgo test files were added or modified, so the check is not applicable.
Test Structure And Quality ✅ Passed PR contains no Ginkgo test code modifications. All changes are documentation and configuration files (docs, .apm, .claude, .coderabbit.yaml, etc.), making the test structure review not applicable.
Microshift Test Compatibility ✅ Passed PR contains only documentation and configuration changes (MD, YAML, JSON files). No new Ginkgo e2e tests are added; check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The PR contains only documentation and configuration changes (markdown docs, yaml/json configs). No Ginkgo e2e tests (It(), Describe(), Context(), When(), etc.) are being added.
Topology-Aware Scheduling Compatibility ✅ Passed PR contains only documentation and configuration changes; no deployment manifests, operator code, or scheduling constraints were introduced.
Ote Binary Stdout Contract ✅ Passed PR contains only documentation and configuration changes (Markdown, YAML, JSON); no Go source code or OTE binary code is modified, making the OTE stdout contract check inapplicable.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR adds documentation and configuration, not Ginkgo e2e tests. Standard Go tests added lack Ginkgo patterns. Check not applicable.
No-Weak-Crypto ✅ Passed This PR contains only documentation and configuration changes (markdown, YAML, JSON files) with no code additions. No weak cryptographic algorithms (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custo...
Container-Privileges ✅ Passed PR contains no privileged container configurations. All modified Dockerfiles lack privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN capabilities, and allowPrivilegeEscalation settings. Two...
Title check ✅ Passed The title 'add feature docs for symptoms' accurately describes the main change: adding feature documentation for the symptoms feature, as evidenced by the new docs/features/job-analysis-symptoms.md file and related documentation rule updates.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 10, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
docs/features/job-analysis-symptoms.md (1)

61-67: 💤 Low value

Add language specifier to fenced code block.

The ASCII diagram should specify a language identifier (e.g., text) to satisfy linting rules and improve rendering consistency.

🎨 Suggested improvement
-```
+```text
 Symptom  ──matches──▶  job artifact
    │                      │
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/features/job-analysis-symptoms.md` around lines 61 - 67, The fenced
ASCII diagram block lacks a language specifier; update the markdown code fence
that contains "Symptom  ──matches──▶  job artifact" / "Label  ◀──recorded on──
job run (BQ + GCS + Postgres)" to include a language tag (e.g., use ```text
instead of ``` ) so the block is lint-compliant and renders consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.coderabbit.yaml:
- Line 154: The YAML sequence item "- name: \"Feature Documentation\"" has one
extra leading space causing a syntax error; fix it by reducing its indentation
to match the other sequence entries (ensure the "- name: ..." line uses the same
number of leading spaces as the other "- name" items in the same block) so the
YAML parser accepts the block.
- Around line 68-70: Update the wording in the .coderabbit.yaml instructions
block so it refers to a "file" instead of a "directory" for the path pattern
matching sippy-ng/src/component_readiness/JobArtifactQuery.js; locate the
instructions: | entry and change "This directory is part of the symptoms
feature..." to "This file is part of the symptoms feature..." (and adjust any
similar occurrences in that same instructions text).

In `@docs/features/job-analysis-symptoms.md`:
- Around line 50-54: The docs list the matcher type as "none" but the validation
in pkg/api/jobrunscan/symptoms.go expects the type "file"; update the
documentation entry (the bullet currently saying `none` — glob file pattern
only) to use `file` instead and ensure any other occurrences of the `none`
matcher in this document are replaced so the doc matches the validator's
accepted matcher types (which include the string literal "file").

---

Nitpick comments:
In `@docs/features/job-analysis-symptoms.md`:
- Around line 61-67: The fenced ASCII diagram block lacks a language specifier;
update the markdown code fence that contains "Symptom  ──matches──▶  job
artifact" / "Label  ◀──recorded on── job run (BQ + GCS + Postgres)" to include a
language tag (e.g., use ```text instead of ``` ) so the block is lint-compliant
and renders consistently.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f0127b5a-311e-4639-bcb2-0bdcfca08882

📥 Commits

Reviewing files that changed from the base of the PR and between f9c9ec2 and 971a389.

📒 Files selected for processing (3)
  • .apm/instructions/docs.instructions.md
  • .coderabbit.yaml
  • docs/features/job-analysis-symptoms.md

Comment thread .coderabbit.yaml
Comment thread .coderabbit.yaml Outdated
Comment thread docs/features/job-analysis-symptoms.md Outdated
@sosiouxme sosiouxme force-pushed the 20260605-retroactive-symptoms branch from 971a389 to b6333a1 Compare June 10, 2026 23:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
.coderabbit.yaml (1)

150-150: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix YAML indentation to prevent syntax error.

Line 150 has 7 spaces before the dash, but the YAML block sequence requires 6 spaces (consistent with lines 77, 86, 95, 113, 128). This extra space causes the YAML parser to fail and prevents the configuration file from loading.

🐛 Proposed fix
-       - name: "Feature Documentation"
+      - name: "Feature Documentation"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.coderabbit.yaml at line 150, The YAML has one extra leading space before
the sequence item "- name: \"Feature Documentation\"" causing a parse error;
locate the sequence item whose key is name: "Feature Documentation" in
.coderabbit.yaml and remove one leading space so the dash is indented with the
same 6-space prefix as the other block sequence items, preserving consistent
indentation for the YAML list.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In @.coderabbit.yaml:
- Line 150: The YAML has one extra leading space before the sequence item "-
name: \"Feature Documentation\"" causing a parse error; locate the sequence item
whose key is name: "Feature Documentation" in .coderabbit.yaml and remove one
leading space so the dash is indented with the same 6-space prefix as the other
block sequence items, preserving consistent indentation for the YAML list.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c73c4844-3b34-4ae6-b50e-fcb1d46bd819

📥 Commits

Reviewing files that changed from the base of the PR and between 971a389 and b6333a1.

📒 Files selected for processing (6)
  • .apm/instructions/docs.instructions.md
  • .coderabbit.yaml
  • .mcp.json
  • AGENTS.md
  • CLAUDE.md
  • docs/features/job-analysis-symptoms.md
✅ Files skipped from review due to trivial changes (4)
  • .mcp.json
  • AGENTS.md
  • .apm/instructions/docs.instructions.md
  • CLAUDE.md

@sosiouxme sosiouxme force-pushed the 20260605-retroactive-symptoms branch from b6333a1 to 070cc00 Compare June 10, 2026 23:42

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/features/job-analysis-symptoms.md`:
- Around line 59-65: The fenced ASCII-art code block in
docs/features/job-analysis-symptoms.md is missing a language identifier which
triggers markdownlint; update the opening fence from ``` to ```text (or
```plaintext) so the diagram block begins with a language specifier, e.g. change
the fence around the lines starting "Symptom  ──matches──▶  job artifact" to use
```text to silence the lint warning.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 7cf46872-1aa5-4bc0-99cd-37d63f787d45

📥 Commits

Reviewing files that changed from the base of the PR and between b6333a1 and 070cc00.

⛔ Files ignored due to path filters (2)
  • AGENTS.md is excluded by !AGENTS.md
  • CLAUDE.md is excluded by !CLAUDE.md
📒 Files selected for processing (4)
  • .apm/instructions/docs.instructions.md
  • .coderabbit.yaml
  • .mcp.json
  • docs/features/job-analysis-symptoms.md
✅ Files skipped from review due to trivial changes (3)
  • .apm/instructions/docs.instructions.md
  • .mcp.json
  • .coderabbit.yaml

Comment thread docs/features/job-analysis-symptoms.md Outdated
🤖 Assisted by RH MPEX Chai-bot
@sosiouxme sosiouxme force-pushed the 20260605-retroactive-symptoms branch from 070cc00 to 4fdabc5 Compare June 11, 2026 00:44
@sosiouxme sosiouxme closed this Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant