Add article quality validation gate to generate-news-enhanced.ts pipeline#469
Add article quality validation gate to generate-news-enhanced.ts pipeline#469
Conversation
…line Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
github-actions
bot
added
testing
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
There was a problem hiding this comment.
Pull request overview
This pull request adds a comprehensive article quality validation gate to the generate-news-enhanced.ts pipeline to prevent low-quality articles from being committed. The implementation introduces a scoring system that evaluates four key dimensions (word count, unknown authors, untranslated spans, and analytical structure) and provides visibility into article quality through console reports and CI/CD-compatible exit codes.
Changes:
- Added
validateArticleQuality()function with a 100-point scoring system across 4 dimensions (25 points each) - Integrated quality validation into the
writeSingleArticle()function with detailed console reporting - Added
--quality-thresholdCLI flag (default: 40) for CI/CD configuration - Implemented exit code 2 for soft failures when all articles score below threshold
- Added comprehensive unit tests (17 test cases) covering all scoring dimensions and edge cases
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scripts/generate-news-enhanced.ts |
Added validateArticleQuality function, integrated quality reporting into writeSingleArticle, added CLI flag parsing for quality threshold, implemented exit code 2 logic for quality gate failures |
tests/article-quality.test.ts |
Added comprehensive unit tests covering word count scoring, unknown author detection, untranslated span detection, analytical section counting, and pass/fail thresholds |
scripts/generate-news-enhanced.ts
Outdated
| articleId: string; | ||
| wordCount: number; | ||
| unknownAuthorCount: number; | ||
| totalEntryCount: number; | ||
| untranslatedSpanCount: number; | ||
| analyticalSectionCount: number; | ||
| score: number; | ||
| passed: boolean; | ||
| issues: string[]; |
There was a problem hiding this comment.
The ArticleQualityReport interface properties should be marked as readonly for consistency with the test interface and immutability best practices. The test file defines these properties as readonly (lines 19-27), but the implementation does not (lines 421-429).
| articleId: string; | |
| wordCount: number; | |
| unknownAuthorCount: number; | |
| totalEntryCount: number; | |
| untranslatedSpanCount: number; | |
| analyticalSectionCount: number; | |
| score: number; | |
| passed: boolean; | |
| issues: string[]; | |
| readonly articleId: string; | |
| readonly wordCount: number; | |
| readonly unknownAuthorCount: number; | |
| readonly totalEntryCount: number; | |
| readonly untranslatedSpanCount: number; | |
| readonly analyticalSectionCount: number; | |
| readonly score: number; | |
| readonly passed: boolean; | |
| readonly issues: string[]; |
There was a problem hiding this comment.
Fixed in c36cb05 — all 9 ArticleQualityReport properties are now readonly.
scripts/generate-news-enhanced.ts
Outdated
| * | ||
| * @param html - Full HTML content of the article | ||
| * @param lang - Language code (e.g. 'en', 'sv') | ||
| * @param articleType - Article type label for reporting (e.g. 'motions-en') |
There was a problem hiding this comment.
The JSDoc example for the articleType parameter is misleading. It shows 'motions-en' but the actual usage (lines 717, 804, 888, 972) passes just the article type without language code (e.g., 'motions', 'propositions'). The language code is appended internally at line 447 to form the articleId. The example should be updated to 'motions' instead of 'motions-en'.
| * @param articleType - Article type label for reporting (e.g. 'motions-en') | |
| * @param articleType - Article type label for reporting (e.g. 'motions') |
There was a problem hiding this comment.
Fixed in c36cb05 — JSDoc example updated to 'motions'.
|
@copilot apply changes based on the comments in this thread |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
…ample Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Done in c36cb05 — both review suggestions applied: |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
…t.ts Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| // Use list items as a proxy for total document entries | ||
| const listItemCount = (html.match(/<li[^>]*>/g) ?? []).length; | ||
| const totalEntryCount = Math.max(listItemCount, unknownAuthorCount); | ||
|
|
There was a problem hiding this comment.
totalEntryCount is derived from a raw <li> count, but the generated articles use <div class="*-entry"> blocks and class="document-link" anchors for documents (and <li> also appears in the event calendar/watch lists). This will skew the unknown-author ratio (e.g., known entries may not be counted at all, or unrelated <li>s may dilute the ratio), leading to incorrect scores/exit-code behavior. Consider counting document entries via a more stable marker like class="document-link" (or per-articleType entry class) instead of <li>.
| // Use list items as a proxy for total document entries | |
| const listItemCount = (html.match(/<li[^>]*>/g) ?? []).length; | |
| const totalEntryCount = Math.max(listItemCount, unknownAuthorCount); | |
| // Prefer explicit document links as a proxy for total document entries | |
| const documentLinkCount = (html.match(/class=["']document-link["']/g) ?? []).length; | |
| // Fallback: use list items when no explicit document links are present | |
| const listItemCount = (html.match(/<li[^>]*>/g) ?? []).length; | |
| const baseEntryCount = documentLinkCount > 0 ? documentLinkCount : listItemCount; | |
| // Ensure total entries are never less than the number of unknown-author entries | |
| const totalEntryCount = Math.max(baseEntryCount, unknownAuthorCount); |
| } | ||
|
|
||
| const score = wordScore + unknownScore + untranslatedScore + analyticalScore; | ||
| const passed = score >= qualityThreshold; |
There was a problem hiding this comment.
Articles flagged with Word count: < 300 — REJECT can still end up with passed=true if the other dimensions score high, because passed only checks score >= qualityThreshold. This contradicts the “REJECT” semantics and can prevent the run-level soft-failure from triggering. Consider forcing passed=false (and optionally treating the score as below threshold) whenever the reject condition is hit (e.g., wordCount < 300).
| const passed = score >= qualityThreshold; | |
| const hardRejected = wordCount < 300; | |
| const passed = !hardRejected && score >= qualityThreshold; |
| const qualityThreshold: number = qualityThresholdArg ? parseInt(qualityThresholdArg.split('=')[1] ?? '40', 10) : 40; | ||
|
|
There was a problem hiding this comment.
--quality-threshold= parsing can yield NaN (e.g., empty value --quality-threshold= or non-numeric input). A NaN threshold makes all score >= qualityThreshold comparisons false and also prevents the every(s < qualityThreshold) soft-failure check from ever triggering. Consider validating the parsed value (e.g., Number.isFinite) and falling back to the default or exiting with a clear error when invalid.
| const qualityThreshold: number = qualityThresholdArg ? parseInt(qualityThresholdArg.split('=')[1] ?? '40', 10) : 40; | |
| const DEFAULT_QUALITY_THRESHOLD = 40; | |
| const qualityThresholdRaw: string | undefined = qualityThresholdArg ? qualityThresholdArg.split('=')[1] : undefined; | |
| let qualityThreshold: number = DEFAULT_QUALITY_THRESHOLD; | |
| if (qualityThresholdRaw !== undefined && qualityThresholdRaw !== '') { | |
| const parsedQuality = Number(qualityThresholdRaw); | |
| if (Number.isFinite(parsedQuality)) { | |
| qualityThreshold = parsedQuality; | |
| } else { | |
| console.error( | |
| `Invalid --quality-threshold value "${qualityThresholdRaw}". Using default ${DEFAULT_QUALITY_THRESHOLD}.`, | |
| ); | |
| } | |
| } |
| export function validateArticleQuality(html: string, lang: string, articleType: string): ArticleQualityReport { | ||
| const articleId = `${articleType}-${lang}`; | ||
|
|
||
| // Word count: strip tags and count whitespace-separated tokens | ||
| const textContent = html.replace(/<[^>]+>/g, ' ').replace(/\s+/g, ' ').trim(); | ||
| const wordCount = textContent.length === 0 ? 0 : textContent.split(' ').filter(w => w.length > 0).length; | ||
|
|
||
| // Count "Unknown (Unknown)" sentinel entries (used when author/party is missing) | ||
| const unknownAuthorCount = (html.match(/Unknown \(Unknown\)/g) ?? []).length; | ||
|
|
||
| // Use list items as a proxy for total document entries | ||
| const listItemCount = (html.match(/<li[^>]*>/g) ?? []).length; | ||
| const totalEntryCount = Math.max(listItemCount, unknownAuthorCount); | ||
|
|
||
| // Count untranslated spans — only relevant for non-Swedish content | ||
| const untranslatedSpanCount = lang !== 'sv' | ||
| ? (html.match(/data-translate="true"/g) ?? []).length | ||
| : 0; | ||
|
|
||
| // Count analytical h2 sections (structural quality indicator) | ||
| const analyticalSectionCount = (html.match(/<h2[^>]*>/g) ?? []).length; | ||
|
|
||
| const issues: string[] = []; | ||
|
|
||
| // Word count score: 25 pts | ||
| let wordScore = 0; | ||
| if (wordCount >= 500) { | ||
| wordScore = 25; | ||
| } else if (wordCount >= 300) { | ||
| wordScore = 15; | ||
| } else { | ||
| issues.push(`Word count: ${wordCount} < 300 — REJECT`); | ||
| } | ||
|
|
||
| // Unknown authors score: 25 pts | ||
| let unknownScore = 0; | ||
| const unknownRatio = totalEntryCount > 0 ? unknownAuthorCount / totalEntryCount : 0; | ||
| if (unknownRatio === 0) { | ||
| unknownScore = 25; | ||
| } else if (unknownRatio <= 0.5) { | ||
| unknownScore = Math.round(25 * (1 - unknownRatio)); | ||
| issues.push(`Unknown authors: ${unknownAuthorCount}/${totalEntryCount} ⚠️`); | ||
| } else { | ||
| issues.push(`Unknown authors: ${unknownAuthorCount}/${totalEntryCount} ⚠️`); | ||
| } | ||
|
|
||
| // Untranslated spans score: 25 pts | ||
| let untranslatedScore = 0; | ||
| if (lang === 'sv' || untranslatedSpanCount === 0) { | ||
| untranslatedScore = 25; | ||
| } else if (untranslatedSpanCount <= 10) { | ||
| untranslatedScore = Math.round(25 * (1 - untranslatedSpanCount / 10)); | ||
| issues.push(`Untranslated spans: ${untranslatedSpanCount} ⚠️`); | ||
| } else { | ||
| issues.push(`Untranslated spans: ${untranslatedSpanCount} > 10 ⚠️`); | ||
| } | ||
|
|
||
| // Analytical sections score: 25 pts | ||
| let analyticalScore = 0; | ||
| if (analyticalSectionCount >= 3) { | ||
| analyticalScore = 25; | ||
| } else if (analyticalSectionCount >= 1) { | ||
| analyticalScore = Math.round(25 * analyticalSectionCount / 3); | ||
| issues.push(`Analytical sections: ${analyticalSectionCount}/3 ⚠️`); | ||
| } else { | ||
| issues.push(`Analytical sections: 0/3 ⚠️`); | ||
| } | ||
|
|
||
| const score = wordScore + unknownScore + untranslatedScore + analyticalScore; | ||
| const passed = score >= qualityThreshold; | ||
|
|
There was a problem hiding this comment.
validateArticleQuality is exported but its passed value depends on the module-level qualityThreshold parsed from CLI args at import time. This makes the function’s behavior context-dependent for external callers (and for tests/tools importing the module), which is surprising for a reusable API. Consider making the threshold an explicit parameter (or returning score/metrics only and letting callers decide pass/fail).
| async function writeSingleArticle(html: string, slug: string, lang: Language, articleType?: string): Promise<string> { | ||
| // Validate article quality before writing | ||
| const report = validateArticleQuality(html, lang, articleType ?? slug); | ||
| const unknownRatioStr = report.totalEntryCount > 0 | ||
| ? `${report.unknownAuthorCount}/${report.totalEntryCount}` | ||
| : `${report.unknownAuthorCount}/0`; | ||
| const unknownIcon = report.unknownAuthorCount > 0 ? '⚠️' : '✅'; | ||
| const untranslatedIcon = report.untranslatedSpanCount > 0 ? '⚠️' : '✅'; | ||
| const analyticalIcon = report.analyticalSectionCount >= 3 ? '✅' : '⚠️'; | ||
| console.log(`\n 📊 Article Quality Report: ${report.articleId}`); | ||
| console.log(` - Word count: ${report.wordCount} ${report.wordCount >= 300 ? '✅' : '❌'}`); | ||
| console.log(` - Unknown authors: ${unknownRatioStr} ${unknownIcon}`); | ||
| console.log(` - Untranslated spans: ${report.untranslatedSpanCount} ${untranslatedIcon}`); | ||
| console.log(` - Analytical sections: ${report.analyticalSectionCount}/3 ${analyticalIcon}`); | ||
| console.log(` - Quality Score: ${report.score}/100 — ${report.passed ? 'ABOVE THRESHOLD' : 'BELOW THRESHOLD'}`); | ||
| if (report.issues.length > 0) { | ||
| report.issues.forEach(issue => console.warn(` ⚠️ ${issue}`)); | ||
| } | ||
| qualityScores.push(report.score); | ||
|
|
||
| const filename: string = `${slug}-${lang}.html`; | ||
| await writeArticle(html, filename); |
There was a problem hiding this comment.
Quality validation currently runs only via writeSingleArticle, but several article types in this pipeline are generated by news-types/* modules that call the provided writeArticle(html, filename) callback directly (e.g., month-ahead/weekly-review/monthly-review/breaking). Those outputs won’t be validated and won’t contribute to qualityScores, so the new gate/exit-code behavior is incomplete relative to the PR’s stated goal. Consider moving validation into writeArticle (deriving lang/articleType from the filename/slug) or providing a validating wrapper callback to those generators.
| /** Shape of the quality report returned by validateArticleQuality */ | ||
| interface ArticleQualityReport { | ||
| readonly articleId: string; | ||
| readonly wordCount: number; | ||
| readonly unknownAuthorCount: number; | ||
| readonly totalEntryCount: number; | ||
| readonly untranslatedSpanCount: number; | ||
| readonly analyticalSectionCount: number; | ||
| readonly score: number; | ||
| readonly passed: boolean; | ||
| readonly issues: string[]; | ||
| } | ||
|
|
||
| /** Partial shape of the generate-news-enhanced module we need for these tests */ | ||
| interface GenerateNewsEnhancedModule { | ||
| readonly validateArticleQuality: (html: string, lang: string, articleType: string) => ArticleQualityReport; | ||
| } |
There was a problem hiding this comment.
This test re-declares the ArticleQualityReport shape locally instead of importing the exported type from generate-news-enhanced. That can drift over time (tests still compiling while the real interface changes). Prefer import type { ArticleQualityReport } from the module (or ReturnType<typeof validateArticleQuality>), and use it in GenerateNewsEnhancedModule.
|
@copilot apply changes based on the comments in this thread |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
Articles were being written to
news/with no quality gate, resulting in committed files with 100%Unknown (Unknown)authors, 21+ untranslateddata-translatespans, and sub-300-word document lists with zero analytical structure.Changes
scripts/generate-news-enhanced.tsvalidateArticleQuality(html, lang, articleType): ArticleQualityReport— 100-point scoring across 4 dimensions (25pts each): word count, unknown-author ratio, untranslated spans (non-svonly), analytical<h2>sectionswriteSingleArticlecalls validation before writing and logs a per-article quality report to console--quality-threshold=NCLI flag (default: 40); exit code 2 when all articles in a run score below threshold (soft monitoring failure)ArticleQualityReportinterface withreadonlyproperties exported for external usetests/article-quality.test.ts17 unit tests covering all scoring dimensions, partial-score boundaries, Swedish span exemption, pass/fail threshold, and
articleIdformat.tests/generate-news-enhanced-mcp-abort.test.tsFixed missing
beforeEachimport causingTS2304TypeScript compile error.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.