Skip to content

feat(ci): tighten PR template + add soft-fail quality gates (#965)#1001

Merged
senamakel merged 10 commits intotinyhumansai:mainfrom
oxoxDev:feat/965-pr-template-ci-guard
Apr 29, 2026
Merged

feat(ci): tighten PR template + add soft-fail quality gates (#965)#1001
senamakel merged 10 commits intotinyhumansai:mainfrom
oxoxDev:feat/965-pr-template-ci-guard

Conversation

@oxoxDev
Copy link
Copy Markdown
Contributor

@oxoxDev oxoxDev commented Apr 28, 2026

Summary

  • Replace the existing 5-item PR Submission Checklist with a 6-item schema-locked version that mandates failure-path tests, matrix sync, feature-ID listing, mock-policy compliance, manual-smoke updates, and `Closes #NNN`.
  • Add `.github/workflows/pr-quality.yml` with three soft-fail jobs (checklist-guard, coverage-matrix, markdown-link-check via lychee) — each `continue-on-error: true` for a 2-week soak window, label-skip on `docs` / `chore`.
  • Land Node-stdlib parsers + CLI shims under `scripts/` plus 7 vitest cases covering them.

Problem

#773 / PR #980 landed the test coverage matrix and testing strategy but left two contracts unenforced:

  1. PR template — checklist items were prose, not parseable. Reviewers had no mechanical signal that the contributor had thought about coverage.
  2. Coverage matrix — `docs/TEST-COVERAGE-MATRIX.md` had no CI gate, so adds / removes / renames could drift silently from the strategy doc.

Plus markdown link rot: the strategy doc references `docs/RELEASE-MANUAL-SMOKE.md` (#971) and the new domain READMEs (#966), and we had no link-check job.

Solution

PR template (`.github/PULL_REQUEST_TEMPLATE.md`)

6 schema-locked items with explicit N/A guidance: tests added, matrix updated, feature IDs listed, no new external network deps, manual smoke updated if release-cut surface, `Closes #NNN` keyword used.

CI workflow (`.github/workflows/pr-quality.yml`)

Three jobs running on `ubuntu-latest`, all `continue-on-error: true`:

  • checklist-guard — runs `scripts/check-pr-checklist.mjs` against `PR_BODY`; non-zero if any non-N/A item is unchecked.
  • coverage-matrix — runs `scripts/check-coverage-matrix.mjs` validating row format, ID regex `^\d+(?:.\d+){2,3}$` (covers 3+4-component IDs — section 3.3 has 9 four-component leaves), status legend (✅/🟡/❌/🚫), no duplicates, no catalog gaps.
  • markdown-link-check — `lycheeverse/lychee-action@v2` with glob `'docs//*.md' 'src//README.md' '.github/PULL_REQUEST_TEMPLATE.md'` so [Docs] README sweep for foundational core domains #966 domain READMEs and [Test] Release-cut manual smoke checklist for OS-level features #971 manual-smoke doc inherit the gate.

All three skip when the PR has the `docs` or `chore` label so trivial doc edits don't block.

Pure parsers + CLI shims

  • `scripts/lib/checklist-parser.mjs` — parses GitHub markdown checklists, treats `N/A:` items as satisfied.
  • `scripts/lib/coverage-matrix-parser.mjs` — parses matrix rows, validates IDs + status legend.
  • `scripts/check-pr-checklist.mjs` + `scripts/check-coverage-matrix.mjs` — CLI shims.
  • `scripts/feature-ids.json` — 136-ID catalog (sorted) derived from the matrix.
  • `app/test/scripts/{checklist,coverage-matrix}-parser.test.ts` — 5 + 2 vitest cases including 4-component ID coverage.

Node stdlib only — no new `package.json` dependencies.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) — 7 vitest cases including duplicate-ID + invalid-status + missing-from-catalog detection
  • Coverage matrix updated — N/A: this PR is the matrix-sync gate itself; no feature rows added/removed/renamed
  • All affected feature IDs from the matrix are listed in the PR description under `## Related` — N/A: not a feature change
  • No new external network dependencies introduced — Node stdlib only
  • Manual smoke checklist updated if this touches release-cut surfaces — N/A: no release-cut surface changes
  • Linked issue closed via `Closes #NNN`

Impact

  • Runtime / build: zero. CI workflow + scripts + 2 vitest specs only.
  • CI cost: 3 new jobs per PR, all soft-fail, all skipped on `docs`/`chore` labels. Lychee runs on a small file set (`docs//*.md`, `src//README.md`, the template).
  • Reviewer effort: gates run async; checklist-guard surfaces unchecked non-N/A items as failures so reviewers can point at them concretely.
  • Soft-fail soak: `continue-on-error: true` for ~2 weeks while we tune the parsers and matrix gates. Flip to hard-fail once stable.

Related

Summary by CodeRabbit

  • Chores

    • Updated the pull request submission template with a standardized checklist and explicit guidance for tests, coverage matrix updates, N/A reasons, smoke steps, and issue-closing links
    • Added an automated PR-quality check that validates submission checklists, verifies coverage matrix sync, and runs documentation link checks
  • New Features

    • Added a machine-readable feature-ID artifact to support coverage validation tooling
  • Tests

    • Added tests for checklist parsing and coverage-matrix validation systems

@oxoxDev oxoxDev requested a review from a team April 28, 2026 12:15
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aa90a4bb-5533-4b2a-a3ad-acb251b4f66b

📥 Commits

Reviewing files that changed from the base of the PR and between 6152b91 and c57109e.

📒 Files selected for processing (1)
  • .github/workflows/pr-quality.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/pr-quality.yml

📝 Walkthrough

Walkthrough

Adds PR template tightening, a soft-fail "PR Quality" GitHub Actions workflow, parser libraries and CLI scripts to validate PR checklists and the test coverage matrix, unit tests for parsers, and an auto-generated feature ID catalog JSON.

Changes

Cohort / File(s) Summary
PR Template & Workflow
\.github/PULL_REQUEST_TEMPLATE.md, \.github/workflows/pr-quality.yml
Replaced checklist content with a standardized checklist requiring N/A: <reason> when applicable; added a PR Quality (soft) workflow with three non-blocking jobs (PR checklist, coverage-matrix check, markdown link check) and label-based skips.
Checklist Parser & CLI
scripts/lib/checklist-parser.mjs, scripts/check-pr-checklist.mjs, app/test/checklist-parser.test.ts
New parser exports parseChecklist and summarize; CLI reads PR_BODY, summarizes and exits non-zero when actionable unchecked items exist; Vitest tests for empty input, N/A handling, code-fence skipping, and case-insensitive checks.
Coverage Matrix Parser & CLI
scripts/lib/coverage-matrix-parser.mjs, scripts/check-coverage-matrix.mjs, app/test/coverage-matrix-parser.test.ts
New parser exports parseMatrix and validateAgainstCatalog; CLI loads matrix and scripts/feature-ids.json, reports parse errors, missing IDs, duplicates, and exits non-zero on failures; tests cover parsing, invalid statuses, duplicates, and catalog validation.
Feature ID Catalog (generated)
scripts/feature-ids.json
New auto-generated JSON listing feature IDs, schema note, and generation timestamp consumed by coverage validation script.

Sequence Diagram(s)

sequenceDiagram
    participant Author
    participant GitHub as "GitHub PR"
    participant Actions as "PR Quality Workflow"
    participant Checklist as "check-pr-checklist.mjs"
    participant Matrix as "check-coverage-matrix.mjs"
    participant Lychee as "markdown link checker"

    Author->>GitHub: Open/Update PR with body
    GitHub->>Actions: Trigger workflow (labels/edit)
    Actions->>Checklist: Run checklist job (PR_BODY)
    Checklist-->>Actions: Summary + exit code
    Actions->>Matrix: Run coverage-matrix job
    Matrix->>Matrix: load TEST-COVERAGE-MATRIX.md + feature-ids.json
    Matrix-->>Actions: Validation report + exit code
    Actions->>Lychee: Run markdown link checks
    Lychee-->>Actions: Link check results
    Actions-->>GitHub: Job statuses (soft-fail / continue-on-error)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • senamakel
  • graycyrus

Poem

🐇 I hopped through checkboxes, counted each line,
Found N/A reasons and made the matrix align.
CI hums softly, checks neath the moon,
Features mapped tidy — merge soon, merge soon!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(ci): tighten PR template + add soft-fail quality gates (#965)' clearly summarizes the main changes—template tightening and new CI quality gates—and is concise and specific.
Linked Issues check ✅ Passed The PR addresses all core requirements from #965: tightened checklist template, three CI jobs (checklist-guard, coverage-matrix, markdown-link-check), soft-fail configuration, label-based skip, and parser tests with N/A handling and code-block detection.
Out of Scope Changes check ✅ Passed All changes align with #965 objectives: PR template updates, workflow configuration, parser scripts, feature-ids catalog, and test suite additions are all within scope; no unrelated code or functionality additions detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Review rate limit: 2/5 reviews remaining, refill in 30 minutes and 57 seconds.

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

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 (3)
scripts/lib/coverage-matrix-parser.mjs (1)

15-22: Inconsistent error handling between ID and status validation.

Invalid IDs cause the row to be skipped (line 17: continue), but invalid statuses still push the row to rows (line 22). This means duplicate detection and catalog validation will include rows with invalid statuses. Consider whether this asymmetry is intentional.

♻️ Option: Skip rows with invalid status for consistency
     if (!VALID_STATUS.has(status)) {
       errors.push(`Row ${id}: invalid status "${status}" (must be one of ${[...VALID_STATUS].join(' ')})`);
+      continue;
     }
     rows.push({ id, name, layer, path, status, notes });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/lib/coverage-matrix-parser.mjs` around lines 15 - 22, The code
currently skips rows with invalid IDs (using ID_REGEX test + continue) but still
pushes rows with invalid statuses into rows, causing downstream validation on
bad data; update the status-validation branch around VALID_STATUS and errors so
that when status is invalid you do not push the row (either add a continue after
pushing the status error or move rows.push into the valid-status branch) — refer
to ID_REGEX, VALID_STATUS, errors, and rows to locate and fix the logic.
.github/workflows/pr-quality.yml (1)

13-17: Soft-fail configuration is appropriate for the soak period.

The continue-on-error: true on each job allows failures to surface without blocking PRs. Consider adding a comment or issue reference noting when to flip these to required status checks.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/pr-quality.yml around lines 13 - 17, The job
"checklist-guard" currently sets continue-on-error: true; add a brief inline
comment above or beside continue-on-error referencing the soak period end or an
issue/PR number that tracks switching this to required status checks, e.g.,
mention the target date or issue ID to flip continue-on-error to false, so
maintainers can find and update the continue-on-error setting for
checklist-guard when the soak completes.
scripts/check-coverage-matrix.mjs (1)

9-10: No error handling for missing files.

If docs/TEST-COVERAGE-MATRIX.md or scripts/feature-ids.json doesn't exist, the script will throw an unhandled exception with a cryptic stack trace. Consider wrapping in try-catch for a friendlier CI error message.

🛡️ Proposed error handling
-const matrixMd = await readFile(join(repoRoot, 'docs/TEST-COVERAGE-MATRIX.md'), 'utf8');
-const catalog = JSON.parse(await readFile(join(repoRoot, 'scripts/feature-ids.json'), 'utf8'));
+let matrixMd, catalog;
+try {
+  matrixMd = await readFile(join(repoRoot, 'docs/TEST-COVERAGE-MATRIX.md'), 'utf8');
+} catch (err) {
+  console.error(`Failed to read coverage matrix: ${err.message}`);
+  process.exit(1);
+}
+try {
+  catalog = JSON.parse(await readFile(join(repoRoot, 'scripts/feature-ids.json'), 'utf8'));
+} catch (err) {
+  console.error(`Failed to read/parse feature-ids.json: ${err.message}`);
+  process.exit(1);
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/check-coverage-matrix.mjs` around lines 9 - 10, The two await
readFile calls that populate matrixMd and catalog lack error handling, so
missing files or invalid JSON will throw uncaught exceptions; wrap the file
reads (and JSON.parse for catalog) in a try-catch around the code that calls
readFile and JSON.parse, and on error print a clear, friendly message including
the file name and the error (use the join(repoRoot,
'docs/TEST-COVERAGE-MATRIX.md') and join(repoRoot, 'scripts/feature-ids.json')
paths for context), then call process.exit(1) so CI fails with a readable error;
update the block that assigns matrixMd and catalog to use this try-catch and
ensure JSON.parse errors are surfaced with the same friendly message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/PULL_REQUEST_TEMPLATE.md:
- Line 24: The checklist entry in .github/PULL_REQUEST_TEMPLATE.md referencing
"docs/RELEASE-MANUAL-SMOKE.md" is broken; either add the missing file
docs/RELEASE-MANUAL-SMOKE.md with the required manual smoke checklist content or
update the checklist line in .github/PULL_REQUEST_TEMPLATE.md to reference an
existing document (e.g., replace the path or remove the checkbox) so the link no
longer causes CI to fail; ensure the updated checklist text exactly replaces the
current "- [ ] Manual smoke checklist updated if this touches release-cut
surfaces ([`docs/RELEASE-MANUAL-SMOKE.md`](../docs/RELEASE-MANUAL-SMOKE.md))"
entry.

In @.github/workflows/pr-quality.yml:
- Around line 57-66: The Lychee link check step ("Lychee link check") includes
'.github/PULL_REQUEST_TEMPLATE.md' in the args list which references a missing
file 'docs/RELEASE-MANUAL-SMOKE.md', causing the check to fail; fix by either
removing the '.github/PULL_REQUEST_TEMPLATE.md' entry from the args array in the
lychee-action step, updating the PULL_REQUEST_TEMPLATE.md to remove or correct
the broken 'docs/RELEASE-MANUAL-SMOKE.md' link, or add the missing
'docs/RELEASE-MANUAL-SMOKE.md' file so the reference resolves.

In `@scripts/lib/checklist-parser.mjs`:
- Around line 9-17: The parser currently treats every line the same and includes
checklist markers inside fenced code blocks; modify the for loop that iterates
body.split(/\r?\n/) to track an inFence boolean toggled when encountering fence
delimiters (e.g., lines that match /^\s*```/ or /^\s*~~~/) and skip processing
(continue) while inFence is true so ITEM_REGEX and NA_REGEX matching (the
existing match/checked/text/naReason logic and items.push) only runs for lines
outside fenced code blocks.

---

Nitpick comments:
In @.github/workflows/pr-quality.yml:
- Around line 13-17: The job "checklist-guard" currently sets continue-on-error:
true; add a brief inline comment above or beside continue-on-error referencing
the soak period end or an issue/PR number that tracks switching this to required
status checks, e.g., mention the target date or issue ID to flip
continue-on-error to false, so maintainers can find and update the
continue-on-error setting for checklist-guard when the soak completes.

In `@scripts/check-coverage-matrix.mjs`:
- Around line 9-10: The two await readFile calls that populate matrixMd and
catalog lack error handling, so missing files or invalid JSON will throw
uncaught exceptions; wrap the file reads (and JSON.parse for catalog) in a
try-catch around the code that calls readFile and JSON.parse, and on error print
a clear, friendly message including the file name and the error (use the
join(repoRoot, 'docs/TEST-COVERAGE-MATRIX.md') and join(repoRoot,
'scripts/feature-ids.json') paths for context), then call process.exit(1) so CI
fails with a readable error; update the block that assigns matrixMd and catalog
to use this try-catch and ensure JSON.parse errors are surfaced with the same
friendly message.

In `@scripts/lib/coverage-matrix-parser.mjs`:
- Around line 15-22: The code currently skips rows with invalid IDs (using
ID_REGEX test + continue) but still pushes rows with invalid statuses into rows,
causing downstream validation on bad data; update the status-validation branch
around VALID_STATUS and errors so that when status is invalid you do not push
the row (either add a continue after pushing the status error or move rows.push
into the valid-status branch) — refer to ID_REGEX, VALID_STATUS, errors, and
rows to locate and fix the logic.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 033986f8-a05b-492b-a41c-437772c26756

📥 Commits

Reviewing files that changed from the base of the PR and between 1a25f5b and ae09b0c.

📒 Files selected for processing (9)
  • .github/PULL_REQUEST_TEMPLATE.md
  • .github/workflows/pr-quality.yml
  • app/test/scripts/checklist-parser.test.ts
  • app/test/scripts/coverage-matrix-parser.test.ts
  • scripts/check-coverage-matrix.mjs
  • scripts/check-pr-checklist.mjs
  • scripts/feature-ids.json
  • scripts/lib/checklist-parser.mjs
  • scripts/lib/coverage-matrix-parser.mjs

Comment thread .github/PULL_REQUEST_TEMPLATE.md
Comment on lines +57 to +66
- name: Lychee link check
uses: lycheeverse/lychee-action@v2
with:
args: >-
--no-progress
--include-fragments
'docs/**/*.md'
'src/**/README.md'
'.github/PULL_REQUEST_TEMPLATE.md'
fail: true
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the referenced file doesn't exist
ls -la docs/RELEASE-MANUAL-SMOKE.md 2>&1 || echo "File not found (expected)"

Repository: tinyhumansai/openhuman

Length of output: 168


🏁 Script executed:

find . -name "PULL_REQUEST_TEMPLATE.md" -o -name "PULL_REQUEST_TEMPLATE*" | head -20

Repository: tinyhumansai/openhuman

Length of output: 101


🏁 Script executed:

cat .github/PULL_REQUEST_TEMPLATE.md 2>&1 || echo "File not found"

Repository: tinyhumansai/openhuman

Length of output: 1846


Link check includes the template with a broken link.

The glob on line 65 includes .github/PULL_REQUEST_TEMPLATE.md, which contains a reference to docs/RELEASE-MANUAL-SMOKE.md — a file that doesn't exist. The lychee check will fail until that file is created or the reference is removed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/pr-quality.yml around lines 57 - 66, The Lychee link check
step ("Lychee link check") includes '.github/PULL_REQUEST_TEMPLATE.md' in the
args list which references a missing file 'docs/RELEASE-MANUAL-SMOKE.md',
causing the check to fail; fix by either removing the
'.github/PULL_REQUEST_TEMPLATE.md' entry from the args array in the
lychee-action step, updating the PULL_REQUEST_TEMPLATE.md to remove or correct
the broken 'docs/RELEASE-MANUAL-SMOKE.md' link, or add the missing
'docs/RELEASE-MANUAL-SMOKE.md' file so the reference resolves.

Comment thread scripts/lib/checklist-parser.mjs
oxoxDev added a commit to oxoxDev/openhuman that referenced this pull request Apr 28, 2026
…#965)

- checklist-parser: skip checklist markers inside fenced code blocks
  (CodeRabbit found `- [ ]` lines inside ``` fences would falsely match).
- checklist-parser: tighten N/A regex to `(?:\(N\/A\)|N\/A\b)` so
  `(N/A) reason` correctly consumes the closing paren and
  `N/Always` doesn't false-positive.
- coverage-matrix-parser: drop rows with invalid status from `rows[]`
  (was asymmetric — invalid IDs `continue`d but invalid statuses still
  pushed). Fixes double-counting downstream.
- check-coverage-matrix: wrap input reads in try/catch so missing
  files surface a clear message instead of an unhandled rejection.
- pr-quality.yml: comment the 2-week soft-fail soak intent on the
  `continue-on-error: true` block.
- Move vitest specs from app/test/scripts/ to app/test/ — vitest
  config only includes `test/*.test.{ts,tsx}` (not nested), so the
  scripts/ subdir was silently never running. Two cases added in
  this round (fenced-block skip + invalid-status drop) bring the
  total to 9.

Refs tinyhumansai#965
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@senamakel senamakel left a comment

Choose a reason for hiding this comment

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

@oxoxDev can u explore why the actiosn in the PR are failing and get it to pass

oxoxDev and others added 8 commits April 29, 2026 10:24
Replaces the existing 5-item checklist in .github/PULL_REQUEST_TEMPLATE.md
with a 6-item schema-locked version covering test additions (with
failure-path requirement), coverage matrix sync, feature-ID listing,
mock-policy compliance, manual-smoke updates, and Closes-keyword usage.
N/A guidance is mandatory — items must be marked N/A with a one-line
reason rather than deleted.

Refs tinyhumansai#965

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds two pure-Node-stdlib parsers under scripts/lib/:

- checklist-parser.mjs: parses GitHub markdown PR-body checklists into
  {checked, naReason, text} items, treating "N/A: <reason>" as
  satisfied even when unchecked.
- coverage-matrix-parser.mjs: parses docs/TEST-COVERAGE-MATRIX.md rows,
  validates IDs against `^\d+(?:\.\d+){2,3}$` (covers 3- and
  4-component IDs — section 3.3 uses 4-component), validates status
  legend (✅/🟡/❌/🚫), and reports duplicates + catalog gaps.

Refs tinyhumansai#965

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- scripts/check-pr-checklist.mjs reads PR_BODY env, parses, exits
  non-zero if any non-N/A item is unchecked.
- scripts/check-coverage-matrix.mjs reads docs/TEST-COVERAGE-MATRIX.md
  + scripts/feature-ids.json and validates parse errors, missing IDs,
  and duplicates.
- scripts/feature-ids.json catalogs all 136 feature IDs sorted
  numerically (3- and 4-component, derived from the matrix). Acts as
  the source-of-truth for the matrix-sync gate.

Refs tinyhumansai#965

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- app/test/scripts/checklist-parser.test.ts (5 cases): empty body,
  all-checked, unchecked-no-NA, mix with N/A reasons, mixed-case [x]/[X].
- app/test/scripts/coverage-matrix-parser.test.ts (2 cases): three
  valid rows including a 4-component ID; duplicate-ID + invalid-status
  + missing-from-catalog detection.

Imports use `@ts-expect-error` for the relative .mjs import since the
parser modules under scripts/lib/ are pure ESM with no .d.ts.

Refs tinyhumansai#965

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…i#965)

Adds .github/workflows/pr-quality.yml with three jobs all running on
ubuntu-latest, all `continue-on-error: true` for a 2-week soft-fail
window, all skipped when the PR has the `docs` or `chore` label:

- checklist-guard: runs scripts/check-pr-checklist.mjs against the PR
  body via PR_BODY env.
- coverage-matrix: runs scripts/check-coverage-matrix.mjs against the
  matrix + feature-ids catalog.
- markdown-link-check: lycheeverse/lychee-action@v2 with glob
  `'docs/**/*.md' 'src/**/README.md' '.github/PULL_REQUEST_TEMPLATE.md'`
  so tinyhumansai#966 domain READMEs inherit the gate.

Concurrency cancels superseded runs by PR number / head ref.

Closes tinyhumansai#965

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes single-arg arrow-fn parens and adds blank-line after
`vitest` import per project prettier config. No behaviour change.

Refs tinyhumansai#965

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#965)

- checklist-parser: skip checklist markers inside fenced code blocks
  (CodeRabbit found `- [ ]` lines inside ``` fences would falsely match).
- checklist-parser: tighten N/A regex to `(?:\(N\/A\)|N\/A\b)` so
  `(N/A) reason` correctly consumes the closing paren and
  `N/Always` doesn't false-positive.
- coverage-matrix-parser: drop rows with invalid status from `rows[]`
  (was asymmetric — invalid IDs `continue`d but invalid statuses still
  pushed). Fixes double-counting downstream.
- check-coverage-matrix: wrap input reads in try/catch so missing
  files surface a clear message instead of an unhandled rejection.
- pr-quality.yml: comment the 2-week soft-fail soak intent on the
  `continue-on-error: true` block.
- Move vitest specs from app/test/scripts/ to app/test/ — vitest
  config only includes `test/*.test.{ts,tsx}` (not nested), so the
  scripts/ subdir was silently never running. Two cases added in
  this round (fenced-block skip + invalid-status drop) bring the
  total to 9.

Refs tinyhumansai#965
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop unused @ts-expect-error directives and add explicit param types
for .map / .every / .some callbacks. .mjs imports resolve through ts
module resolution without the directive; CI tsc --noEmit was failing on
TS2578 + TS7006 even though local somehow tolerated it.

Refs tinyhumansai#965
@oxoxDev oxoxDev force-pushed the feat/965-pr-template-ci-guard branch from 42a53ee to 6152b91 Compare April 29, 2026 04:57
@oxoxDev
Copy link
Copy Markdown
Contributor Author

oxoxDev commented Apr 29, 2026

@senamakel addressed:

  1. Markdown Link Check — branch was missing docs/RELEASE-MANUAL-SMOKE.md (now in main via docs(release): add manual smoke checklist (#971) #1000). Rebased onto current main, file picked up. Note: 3 other pre-existing link errors remain in main (install.md#apt-debianubuntu fragment, homebrew-openhuman 404, localhost:9222 in webview-integration-playbook.md) — not introduced by this PR; can be cleaned up separately.

  2. Type Check TypeScript — 5 errors in the new vitest specs (test/checklist-parser.test.ts, test/coverage-matrix-parser.test.ts):

    • TS2578 unused @ts-expect-error × 2 — directives weren't actually needed (.mjs imports resolve through ts module resolution)
    • TS7006 implicit any × 3 — added explicit ChecklistItem interface + (e: string) annotations on .map/.every/.some callbacks

Local gates green: pnpm typecheck, pnpm test (986 pass / 4 skip / 1 todo), pnpm format:check, pnpm lint (0 errors).

CI re-running on 6152b919.

…ai#965)

Markdown Link Check was hard-blocking PR merge on 2 pre-existing main
errors unrelated to this PR:
- docs/install.md#apt-debianubuntu — slug mismatch with lychee's
  fragment resolver (heading text contains parens + slash)
- http://localhost:9222 in docs/webview-integration-playbook.md — a
  legit dev-only URL that should never be probed by CI

Add narrow excludes so the gate flags only new dead links introduced
by future PRs. Branch protection will unblock this PR; pre-existing
errors can be cleaned up in a follow-up rather than scope-creeping
this one.

Refs tinyhumansai#965
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/test/coverage-matrix-parser.test.ts (1)

22-40: Add a malformed-ID regression test.

Please add a case asserting that a row like | 1.2 | ... | yields an Invalid ID format error. This will lock the intended ID-validation contract and prevent silent parser regressions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/test/coverage-matrix-parser.test.ts` around lines 22 - 40, Add a new test
that feeds parseMatrix a row with a malformed ID like `| 1.2 | ... |` and
asserts the parser emits an "Invalid ID format" error; specifically call
parseMatrix(md) where md contains the `1.2` row and assert parsed.errors.some(e
=> e.includes('Invalid ID format')) is true (and optionally assert the malformed
row is not present in parsed.rows). Use the existing test style and functions
(parseMatrix, parsed.errors, parsed.rows) to locate where to add this new case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/lib/coverage-matrix-parser.mjs`:
- Around line 1-16: ROW_REGEX is too strict so malformed IDs don't match the row
pattern and never hit the ID_REGEX validation in parseMatrix; change ROW_REGEX
(used by parseMatrix) to capture the first table cell generically (e.g. accept
any non-pipe content) instead of enforcing the dotted-number shape, then keep
the existing ID_REGEX check to push `Invalid ID format: ${id}` into errors when
the captured id fails validation; update the ROW_REGEX constant and ensure
parseMatrix still trims and validates the captured id against ID_REGEX and
records errors for bad IDs.

---

Nitpick comments:
In `@app/test/coverage-matrix-parser.test.ts`:
- Around line 22-40: Add a new test that feeds parseMatrix a row with a
malformed ID like `| 1.2 | ... |` and asserts the parser emits an "Invalid ID
format" error; specifically call parseMatrix(md) where md contains the `1.2` row
and assert parsed.errors.some(e => e.includes('Invalid ID format')) is true (and
optionally assert the malformed row is not present in parsed.rows). Use the
existing test style and functions (parseMatrix, parsed.errors, parsed.rows) to
locate where to add this new case.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4d561acf-ddb9-4a4f-8b73-59e668a2891c

📥 Commits

Reviewing files that changed from the base of the PR and between ae09b0c and 6152b91.

📒 Files selected for processing (9)
  • .github/PULL_REQUEST_TEMPLATE.md
  • .github/workflows/pr-quality.yml
  • app/test/checklist-parser.test.ts
  • app/test/coverage-matrix-parser.test.ts
  • scripts/check-coverage-matrix.mjs
  • scripts/check-pr-checklist.mjs
  • scripts/feature-ids.json
  • scripts/lib/checklist-parser.mjs
  • scripts/lib/coverage-matrix-parser.mjs
✅ Files skipped from review due to trivial changes (3)
  • scripts/feature-ids.json
  • .github/workflows/pr-quality.yml
  • .github/PULL_REQUEST_TEMPLATE.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • scripts/check-pr-checklist.mjs
  • scripts/check-coverage-matrix.mjs
  • scripts/lib/checklist-parser.mjs

Comment on lines +1 to +16
const ROW_REGEX = /^\| (\d+(?:\.\d+){2,3}) \| ([^|]+) \| ([^|]+) \| ([^|]+) \| ([^|]+) \| ([^|]+) \|/;
const ID_REGEX = /^\d+(?:\.\d+){2,3}$/;
const VALID_STATUS = new Set(['✅', '🟡', '❌', '🚫']);

export function parseMatrix(markdown) {
const rows = [];
const errors = [];
if (typeof markdown !== 'string') {
return { rows, errors };
}
for (const line of markdown.split(/\r?\n/)) {
const match = line.match(ROW_REGEX);
if (!match) continue;
const [, id, name, layer, path, status, notes] = match.map((v) => (typeof v === 'string' ? v.trim() : v));
if (!ID_REGEX.test(id)) {
errors.push(`Invalid ID format: ${id}`);
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.

⚠️ Potential issue | 🟠 Major

Malformed IDs are silently dropped instead of reported.

Because Line 1 already requires a valid ID shape, malformed IDs never reach the Line 15 validation branch. That bypasses Invalid ID format errors and weakens the gate.

Proposed fix
-const ROW_REGEX = /^\| (\d+(?:\.\d+){2,3}) \| ([^|]+) \| ([^|]+) \| ([^|]+) \| ([^|]+) \| ([^|]+) \|/;
+const ROW_REGEX = /^\|\s*([0-9][^|]*)\s*\|\s*([^|]+)\s*\|\s*([^|]+)\s*\|\s*([^|]+)\s*\|\s*([^|]+)\s*\|\s*([^|]*)\s*\|$/;
 const ID_REGEX = /^\d+(?:\.\d+){2,3}$/;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const ROW_REGEX = /^\| (\d+(?:\.\d+){2,3}) \| ([^|]+) \| ([^|]+) \| ([^|]+) \| ([^|]+) \| ([^|]+) \|/;
const ID_REGEX = /^\d+(?:\.\d+){2,3}$/;
const VALID_STATUS = new Set(['✅', '🟡', '❌', '🚫']);
export function parseMatrix(markdown) {
const rows = [];
const errors = [];
if (typeof markdown !== 'string') {
return { rows, errors };
}
for (const line of markdown.split(/\r?\n/)) {
const match = line.match(ROW_REGEX);
if (!match) continue;
const [, id, name, layer, path, status, notes] = match.map((v) => (typeof v === 'string' ? v.trim() : v));
if (!ID_REGEX.test(id)) {
errors.push(`Invalid ID format: ${id}`);
const ROW_REGEX = /^\|\s*([0-9][^|]*)\s*\|\s*([^|]+)\s*\|\s*([^|]+)\s*\|\s*([^|]+)\s*\|\s*([^|]+)\s*\|\s*([^|]*)\s*\|$/;
const ID_REGEX = /^\d+(?:\.\d+){2,3}$/;
const VALID_STATUS = new Set(['✅', '🟡', '❌', '🚫']);
export function parseMatrix(markdown) {
const rows = [];
const errors = [];
if (typeof markdown !== 'string') {
return { rows, errors };
}
for (const line of markdown.split(/\r?\n/)) {
const match = line.match(ROW_REGEX);
if (!match) continue;
const [, id, name, layer, path, status, notes] = match.map((v) => (typeof v === 'string' ? v.trim() : v));
if (!ID_REGEX.test(id)) {
errors.push(`Invalid ID format: ${id}`);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/lib/coverage-matrix-parser.mjs` around lines 1 - 16, ROW_REGEX is too
strict so malformed IDs don't match the row pattern and never hit the ID_REGEX
validation in parseMatrix; change ROW_REGEX (used by parseMatrix) to capture the
first table cell generically (e.g. accept any non-pipe content) instead of
enforcing the dotted-number shape, then keep the existing ID_REGEX check to push
`Invalid ID format: ${id}` into errors when the captured id fails validation;
update the ROW_REGEX constant and ensure parseMatrix still trims and validates
the captured id against ID_REGEX and records errors for bad IDs.

…i#965)

The brew tap repo doesn't exist yet (planned, not created). Pre-existing
in docs/install.md. Add exclude so the gate flags only new dead links.

Refs tinyhumansai#965
@senamakel senamakel merged commit a09222b into tinyhumansai:main Apr 29, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Test Infra] Tighten PR template + add CI enforcement for tests/docs

2 participants