📖 Add AI quality assurance documentation#4359
Conversation
|
/lgtm |
kubestellar-prow
bot
added
the
dco-signoff: yes
|
@clubanderson: you cannot LGTM your own PR. 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 kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: clubanderson The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole canceled.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: clubanderson The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubestellar-prow
bot
added
approved
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
clubanderson
force-pushed
the
docs/ai-quality-assurance
branch
from
e9c2735 to
6cc636d
Compare
There was a problem hiding this comment.
Pull request overview
Adds repository documentation describing AI-assisted development quality gates/CI checks, and links it from top-level contributor-facing docs to address community concerns about regression risk.
Changes:
- Add
docs/AI-QUALITY-ASSURANCE.mddescribing PR checks, post-build safety checks, post-merge verification, and monitoring. - Add a new “Quality Assurance” section to
README.mdlinking to the new document. - Add a reference to the new QA doc from
CONTRIBUTING.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| README.md | Adds a QA overview section and links to the detailed QA document. |
| docs/AI-QUALITY-ASSURANCE.md | New detailed documentation for CI/QA checks and incident response model. |
| CONTRIBUTING.md | Adds a pointer to the QA documentation from contributor guidance. |
| Console uses AI tools (GitHub Copilot, Claude Code) to accelerate development. Quality is maintained through **layered feedback loops** — every PR triggers the same automated checks regardless of author, and continuous monitoring catches what PR checks miss. | ||
|
|
||
| - **Before commit**: TypeScript build + Go build + 5 post-build safety checks + lint | ||
| - **Before merge**: nil-safety, ts-null-safety, array-safety, API contract, Playwright E2E, coverage gate, TTFI performance, CodeQL, Copilot code review |
There was a problem hiding this comment.
The bullet claims ts-null-safety is a "ratcheted baseline" that only allows improvement, but the ts-null-safety job in .github/workflows/nil-safety.yml fails on any findings (no baseline/ratchet). Consider rewording to distinguish: nilaway + array-safety are ratcheted, while ts-null-safety is a strict fail-on-findings check.
| - **Before merge**: nil-safety, ts-null-safety, array-safety, API contract, Playwright E2E, coverage gate, TTFI performance, CodeQL, Copilot code review | |
| - **Before merge**: ratcheted nil-safety and array-safety baselines, strict ts-null-safety (fail on findings), API contract, Playwright E2E, coverage gate, TTFI performance, CodeQL, Copilot code review |
| Console uses AI tools (GitHub Copilot, Claude Code) to accelerate development. Quality is maintained through **layered feedback loops** — every PR triggers the same automated checks regardless of author, and continuous monitoring catches what PR checks miss. | ||
|
|
||
| - **Before commit**: TypeScript build + Go build + 5 post-build safety checks + lint | ||
| - **Before merge**: nil-safety, ts-null-safety, array-safety, API contract, Playwright E2E, coverage gate, TTFI performance, CodeQL, Copilot code review |
There was a problem hiding this comment.
This says the API contract check runs "against a live demo backend", but api-contract.yml actually builds and starts the backend locally (demo mode) on localhost:8080 and runs the contract script against that. Suggest rephrasing to "against an ephemeral locally-started demo backend" (or similar) to match the workflow behavior.
| - **Before merge**: nil-safety, ts-null-safety, array-safety, API contract, Playwright E2E, coverage gate, TTFI performance, CodeQL, Copilot code review | |
| - **Before merge**: nil-safety, ts-null-safety, array-safety, API contract (against an ephemeral locally-started demo backend), Playwright E2E, coverage gate, TTFI performance, CodeQL, Copilot code review |
| Console uses AI tools (GitHub Copilot, Claude Code) to accelerate development. Quality is maintained through **layered feedback loops** — every PR triggers the same automated checks regardless of author, and continuous monitoring catches what PR checks miss. | ||
|
|
||
| - **Before commit**: TypeScript build + Go build + 5 post-build safety checks + lint | ||
| - **Before merge**: nil-safety, ts-null-safety, array-safety, API contract, Playwright E2E, coverage gate, TTFI performance, CodeQL, Copilot code review |
There was a problem hiding this comment.
This states Playwright runs "on every PR", but .github/workflows/playwright.yml is triggered on push (main/dev) and workflow_dispatch, not pull_request. If PR Playwright coverage is intended, either update the statement or reference the actual PR-triggered workflows (e.g., perf-ttfi, nil-safety, CodeQL, etc.).
| - **Before merge**: nil-safety, ts-null-safety, array-safety, API contract, Playwright E2E, coverage gate, TTFI performance, CodeQL, Copilot code review | |
| - **Before merge**: nil-safety, ts-null-safety, array-safety, API contract, coverage gate, TTFI performance, CodeQL, Copilot code review |
| - **Before commit**: TypeScript build + Go build + 5 post-build safety checks + lint | ||
| - **Before merge**: nil-safety, ts-null-safety, array-safety, API contract, Playwright E2E, coverage gate, TTFI performance, CodeQL, Copilot code review | ||
| - **After merge**: Targeted Playwright tests run against production (`console.kubestellar.io`); failures reopen the original issue | ||
| - **Continuous**: Hourly coverage (12 shards), 4x daily QA, nightly E2E, nightly security scanning, real-time GA4 error tracking |
There was a problem hiding this comment.
"Hourly coverage" is called out here, but .github/workflows/coverage-hourly.yml currently has no schedule trigger—it's run on pushes to main (i.e., after merges). Consider rewording to avoid implying an hourly schedule unless there's an external scheduler not documented here.
| - **Continuous**: Hourly coverage (12 shards), 4x daily QA, nightly E2E, nightly security scanning, real-time GA4 error tracking | |
| - **Continuous**: Post-merge coverage (12 shards), 4x daily QA, nightly E2E, nightly security scanning, real-time GA4 error tracking |
| | `npm run lint` (ESLint) | Style violations, unsafe patterns, unused imports | ~10 seconds | | ||
| | `go build ./...` | Go compilation errors | ~5 seconds | | ||
| | **5 post-build safety checks** | Vendor corruption, missing chunks, MSW leak, bundle size regression, HTML integrity | <1 second (scans built files) | | ||
|
|
There was a problem hiding this comment.
The markdown tables use || at the start of each row, which renders as an extra empty column in most markdown renderers. Update the table rows to start with a single | so the tables render correctly on GitHub.
| The post-build checks (`web/scripts/check-vendor-safety.mjs`) run automatically after every `npm run build` — no separate step needed. | ||
|
|
||
| ### Loop 2: PR Checks (Before Merge) | ||
|
|
||
| Every PR — human or AI — triggers these checks. Maintainers review results before merging. |
There was a problem hiding this comment.
The PR-level checks table should include the Array Safety ratcheted check, which runs as part of .github/workflows/nil-safety.yml (id: array-safety) and can fail PRs on regressions. Right now README mentions it, but this document’s PR-level table omits it.
|
|
||
| Every PR — human or AI — triggers these checks. Maintainers review results before merging. |
There was a problem hiding this comment.
This row implies Playwright E2E runs for PRs via playwright.yml, but that workflow is triggered on push (main/dev) rather than pull_request. Either adjust the workflow reference or reword this row to avoid claiming PR-level coverage that doesn’t currently run.
| Every PR — human or AI — triggers these checks. Maintainers review results before merging. | ||
|
|
There was a problem hiding this comment.
The Coverage Gate is described like a blocking threshold, but coverage-gate.yml explicitly labels below-threshold results as "Warning only — not blocking" and uses continue-on-error. Consider stating that it posts a report and warns when files are <80%, rather than failing the PR.
|
|
||
| | Signal | Frequency | Feedback | | ||
| |--------|-----------|----------| | ||
| | **Coverage hourly** | Every hour | Full test suite across 12 shards. Alerts if coverage drops >5%. | |
There was a problem hiding this comment.
"Coverage hourly" is listed as running every hour, but coverage-hourly.yml is triggered by pushes to main (and manual dispatch) and does not have a schedule: cron. Consider updating the frequency wording to match the actual triggers.
| | **Coverage hourly** | Every hour | Full test suite across 12 shards. Alerts if coverage drops >5%. | | |
| | **Coverage on `main`** | On push to `main` + manual dispatch | Full test suite across 12 shards. Alerts if coverage drops >5%. | |
|
|
||
| **Manual coding PRs are discouraged.** They take significantly longer to complete and review compared to agent-generated code. PRs that miss required patterns (isDemoData wiring, useCardLoadingState, locale strings, marketplace vs console) will be sent back — these are things coding agents catch automatically. | ||
|
|
||
| **All PRs — human or AI — must pass the same 9 hard CI gates before merge.** There is no separate path for AI-generated code. See [docs/AI-QUALITY-ASSURANCE.md](docs/AI-QUALITY-ASSURANCE.md) for the full list of quality gates, post-build safety checks, and our regression response model. |
There was a problem hiding this comment.
This claims there are "9 hard CI gates before merge", which conflicts with this PR’s own QA doc language (advisory checks / no required status checks) and will also drift as workflows change. Consider rewording to avoid a fixed hard-gate count and align with the documented distinction between failing workflows vs GitHub-required status checks.
| **All PRs — human or AI — must pass the same 9 hard CI gates before merge.** There is no separate path for AI-generated code. See [docs/AI-QUALITY-ASSURANCE.md](docs/AI-QUALITY-ASSURANCE.md) for the full list of quality gates, post-build safety checks, and our regression response model. | |
| **All PRs — human or AI — are expected to satisfy the same CI and quality checks.** There is no separate path for AI-generated code. See [docs/AI-QUALITY-ASSURANCE.md](docs/AI-QUALITY-ASSURANCE.md) for the current list of checks, the distinction between advisory workflow failures and GitHub-required status checks, post-build safety checks, and our regression response model. |
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 6 code suggestion(s) and 5 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
clubanderson
force-pushed
the
docs/ai-quality-assurance
branch
from
6cc636d to
91984a5
Compare
Documents layered feedback loops preventing AI-generated regressions. New CI checks in nil-safety.yml (ratcheted baselines — fail only on increase): - Magic numbers in timers (baseline: 8) - No-op test assertions like expect(true).toBe(true) (baseline: 5) - Hardcoded route strings instead of ROUTES.* (baseline: 2) - Cards missing useCardLoadingState unified controls (baseline: 1) - Non-localized user-facing strings (baseline: 167) Updated greetings.yml to link AI Quality Assurance doc in PR and issue welcome messages so new contributors see the quality gates upfront. Files: docs/AI-QUALITY-ASSURANCE.md, README.md, CONTRIBUTING.md, .github/workflows/nil-safety.yml, .github/workflows/greetings.yml, 5 baseline files Signed-off-by: Andrew Anderson <andy@clubanderson.com>
clubanderson
force-pushed
the
docs/ai-quality-assurance
branch
from
91984a5 to
fd67451
Compare
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
All suggestions have been applied. Here's a summary of each change: README.md:
CONTRIBUTING.md:
docs/AI-QUALITY-ASSURANCE.md:
Note: The original PR branch was deleted after merge and repository rules prevent recreating it, so these changes are committed locally on |
Uh oh!
There was an error while loading. Please reload this page.