Externalize news article CSS, add quality framework, and implement workflow coordination#156
Externalize news article CSS, add quality framework, and implement workflow coordination#156
Conversation
…ntified Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
… badges Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| const f = lang.filters; | ||
| const filename = langKey === 'en' ? 'index.html' : `index_${langKey === 'no' ? 'no' : langKey}.html`; | ||
| const mainIndex = langKey === 'en' ? 'index.html' : `index_${langKey === 'no' ? 'no' : langKey}.html`; | ||
| const isRTL = ['ar', 'he'].includes(langKey); |
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
…eduplication Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
| import fs from 'fs'; | ||
| import path from 'path'; | ||
| import { fileURLToPath } from 'url'; | ||
| import { WorkflowStateCoordinator, MCP_CACHE_TTL_SECONDS, SIMILARITY_THRESHOLD } from '../scripts/workflow-state-coordinator.js'; |
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
There was a problem hiding this comment.
Pull request overview
This PR refactors news article CSS architecture from embedded styles (660 lines per article) to a centralized external stylesheet, adds editorial branding with a tagline, implements multi-language discovery features, and introduces quality framework and workflow coordination tools for the news generation system.
Changes:
- Moved ~350 lines of article CSS from embedded
<style>tags to externalstyles.cssunder.news-articlenamespace - Added "The Economist-style" editorial tagline to article headers
- Implemented cross-language article discovery with language badges and "Available in" indicators across all 14 supported languages
- Created workflow state coordinator for MCP query caching and article deduplication
- Added article quality enhancer with analytical depth scoring, party perspective counting, and cross-reference validation
- Added comprehensive test suites (362 lines for workflow coordination, 443 lines for realtime monitor quality)
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| styles.css | Added 350 lines of .news-article styles (lines 8703-9049) for typography, event calendars, watch sections, responsive design, print optimization, and editorial tagline |
| scripts/article-template.js | Removed generateArticleCSS() call, added HTML comment clarifying external CSS use, added site tagline div with editorial branding |
| scripts/generate-news-indexes.js | Added getAllArticlesWithLanguageInfo(), generateLanguageBadge(), and generateAvailableLanguages() functions for cross-language discovery with 14-language support |
| news/index_*.html | Updated all 14 language index files with lang and availableLanguages metadata, language badge rendering, and multi-language discovery UI |
| scripts/workflow-state-coordinator.js | New 338-line module for MCP caching (2-hour TTL), similarity-based deduplication (70% threshold), and workflow coordination |
| scripts/article-quality-enhancer.js | New 369-line module implementing Economist-style quality framework with analytical depth (0-1), party perspectives (4+), cross-references (3+), and quality scoring (0.75+ target) |
| tests/workflow-state-coordinator.test.js | 362 lines of tests covering state management, MCP caching, duplicate detection, similarity calculations (25 test cases) |
| tests/news-realtime-monitor.test.js | 443 lines of tests covering multi-language sync, quality metrics, party counting, cross-referencing (40+ test cases) |
| const availableText = '可用语言'; | ||
| const availableBadges = availableLangs.map(l => { | ||
| const f = {en:'🇬🇧',sv:'🇸🇪',da:'🇩🇰',no:'🇳🇴',fi:'🇫🇮',de:'🇩🇪',fr:'🇫🇷',es:'🇪🇸',nl:'🇳🇱',ar:'🇸🇦',he:'🇮🇱',ja:'🇯🇵',ko:'🇰🇷',zh:'🇨🇳'}[l] || '🌐'; | ||
| return `<span class="lang-badge-sm"><span aria-hidden="true">${f}</span> ${l.toUpperCase()}</span>`; |
There was a problem hiding this comment.
The CSS class lang-badge-sm is used in the JavaScript but is not defined in styles.css. This will result in unstyled language badges in the "Available in" section. You should either:
- Add
.lang-badge-smstyles to styles.css, or - Reuse the existing
.language-badgeclass that's already defined for.news-page
| return `<span class="lang-badge-sm"><span aria-hidden="true">${f}</span> ${l.toUpperCase()}</span>`; | |
| return `<span class="language-badge"><span aria-hidden="true">${f}</span> ${l.toUpperCase()}</span>`; |
There was a problem hiding this comment.
Added .lang-badge-sm and .available-languages CSS styles to styles.css in commit a2f376d. The language badges will now display with proper styling (green theme, borders, appropriate sizing) consistent with the project's design system.
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| // Clean MCP cache (2-hour TTL) | ||
| Object.keys(this.state.mcpQueryCache).forEach(key => { | ||
| const entry = this.state.mcpQueryCache[key]; | ||
| const entryTime = new Date(entry.timestamp).getTime(); | ||
| if (now - entryTime > MCP_CACHE_TTL_SECONDS * 1000) { |
There was a problem hiding this comment.
cleanupExpiredEntries() expires MCP cache entries using the constant MCP_CACHE_TTL_SECONDS rather than each entry’s stored ttl. Since cacheMCPQuery() allows custom TTLs, entries with TTL > 2h can be deleted too early. Use entry.ttl (with a sane default) when deciding expiration here.
| // Clean MCP cache (2-hour TTL) | |
| Object.keys(this.state.mcpQueryCache).forEach(key => { | |
| const entry = this.state.mcpQueryCache[key]; | |
| const entryTime = new Date(entry.timestamp).getTime(); | |
| if (now - entryTime > MCP_CACHE_TTL_SECONDS * 1000) { | |
| // Clean MCP cache using per-entry TTL (default: 2 hours) | |
| Object.keys(this.state.mcpQueryCache).forEach(key => { | |
| const entry = this.state.mcpQueryCache[key]; | |
| const entryTime = new Date(entry.timestamp).getTime(); | |
| const effectiveTtlSeconds = | |
| typeof entry.ttl === 'number' && entry.ttl > 0 | |
| ? entry.ttl | |
| : MCP_CACHE_TTL_SECONDS; | |
| if (now - entryTime > effectiveTtlSeconds * 1000) { |
There was a problem hiding this comment.
Fixed in commit 849b756. Now uses per-entry TTL with fallback to default constant:
const effectiveTtlSeconds =
typeof entry.ttl === 'number' && entry.ttl > 0
? entry.ttl
: MCP_CACHE_TTL_SECONDS;| /** | ||
| * Check if article is duplicate based on similarity | ||
| * | ||
| * @param {string} title - Article title | ||
| * @param {string[]} topics - Article topics | ||
| * @param {string[]} sources - Article sources | ||
| * @returns {Object} { isDuplicate: boolean, matchedArticle: Object|null, similarityScore: number } | ||
| */ | ||
| async checkDuplicateArticle(title, topics = [], sources = []) { | ||
| this.cleanupExpiredEntries(); | ||
|
|
||
| let maxSimilarity = 0; | ||
| let matchedArticle = null; | ||
|
|
||
| for (const recentArticle of this.state.recentArticles) { | ||
| const similarity = this.calculateSimilarity( | ||
| title, | ||
| topics, | ||
| sources, | ||
| recentArticle.title, | ||
| recentArticle.topics, | ||
| recentArticle.mcpQueries | ||
| ); |
There was a problem hiding this comment.
checkDuplicateArticle() and calculateSimilarity() refer to the third parameter as sources, but the implementation compares it against recentArticle.mcpQueries. This API/Doc mismatch makes it easy to pass the wrong data and get incorrect duplicate detection. Align naming and docs (e.g., rename to mcpQueries everywhere, or add a separate sources field in recentArticles).
There was a problem hiding this comment.
Fixed in commit 849b756. Renamed all instances from sources to mcpQueries for consistency:
checkDuplicateArticle(title, topics, mcpQueries)calculateSimilarity(..., mcpQueries1, ..., mcpQueries2)- Updated JSDoc comments to match implementation
scripts/article-template.js
Outdated
| No embedded CSS needed - promotes consistency and maintainability. --> | ||
| <article class="news-article"> | ||
| <header class="article-header"> | ||
| <div class="site-tagline">Latest news and analysis from Sweden's Riksdag. The Economist-style political journalism covering parliament, government, and agencies with systematic transparency.</div> |
There was a problem hiding this comment.
In non-English article pages, the new tagline is still an English sentence but isn’t marked as such. For accessibility (screen readers) and correct language metadata, consider adding lang="en" to the tagline element or providing per-language translations.
| <div class="site-tagline">Latest news and analysis from Sweden's Riksdag. The Economist-style political journalism covering parliament, government, and agencies with systematic transparency.</div> | |
| <div class="site-tagline"${lang !== 'en' ? ' lang="en"' : ''}>Latest news and analysis from Sweden's Riksdag. The Economist-style political journalism covering parliament, government, and agencies with systematic transparency.</div> |
There was a problem hiding this comment.
Fixed in commit 849b756. Added conditional lang="en" attribute for accessibility:
<div class="site-tagline"${lang !== 'en' ? ' lang="en"' : ''}>
scripts/generate-news-indexes.js
Outdated
| const availableText = AVAILABLE_IN_TRANSLATIONS[currentLang] || 'Available in'; | ||
| const badges = languages.map(lang => generateLanguageBadge(lang, isRTL)).join(' '); | ||
|
|
||
| return `<p class="available-languages"${isRTL ? ' dir="ltr"' : ''}><strong>${availableText}:</strong> ${badges}</p>`; |
There was a problem hiding this comment.
generateAvailableLanguages() forces dir="ltr" on the entire <p> when the current UI language is RTL. That will make the translated “Available in” label render left-to-right (incorrect for Arabic/Hebrew). Prefer keeping the paragraph direction RTL and only force LTR on the individual badges/codes.
| return `<p class="available-languages"${isRTL ? ' dir="ltr"' : ''}><strong>${availableText}:</strong> ${badges}</p>`; | |
| return `<p class="available-languages"><strong>${availableText}:</strong> ${badges}</p>`; |
There was a problem hiding this comment.
Fixed in commit 849b756. Removed paragraph-level dir="ltr" to preserve RTL direction for Arabic/Hebrew labels. Individual badges still have dir="ltr" via generateLanguageBadge().
| /** | ||
| * Workflow State Coordinator | ||
| * | ||
| * Manages coordination between multiple news generation workflows to prevent | ||
| * duplicate work and improve efficiency: | ||
| * - news-realtime-monitor.md (2x daily) | ||
| * - news-evening-analysis.md (daily) | ||
| * - news-article-generator.md (various schedules) | ||
| * | ||
| * Features: | ||
| * - MCP query caching (2-hour TTL) | ||
| * - Similarity-based article deduplication (>70% threshold) | ||
| * - Recent article tracking (last 6 hours) | ||
| * - Workflow coordination metadata | ||
| * | ||
| * Usage: | ||
| * import { WorkflowStateCoordinator } from './workflow-state-coordinator.js'; | ||
| * const coordinator = new WorkflowStateCoordinator(); |
There was a problem hiding this comment.
The PR title/description focus on moving article CSS and adding/repairing language badge styles, but this change set also introduces new workflow coordination and article quality-scoring modules plus new test suites and regenerates multiple news indexes. Please align the PR title/description with the actual scope, or split the unrelated features into separate PRs to reduce review/rollback risk.
| if (!fs.existsSync(dir)) { | ||
| fs.mkdirSync(dir, { recursive: true }); | ||
| } | ||
|
|
||
| await coordinator.save(); |
There was a problem hiding this comment.
This test doesn’t actually verify directory creation when missing: it creates the directory if it doesn’t exist, so coordinator.save() never has to create it. To test the behavior, ensure the directory is removed before calling save(), then assert save() recreates it.
| if (!fs.existsSync(dir)) { | |
| fs.mkdirSync(dir, { recursive: true }); | |
| } | |
| await coordinator.save(); | |
| // Ensure any existing test file is removed | |
| if (fs.existsSync(TEST_STATE_FILE)) { | |
| fs.unlinkSync(TEST_STATE_FILE); | |
| } | |
| // Remove the directory to simulate a missing metadata directory | |
| if (fs.existsSync(dir)) { | |
| fs.rmdirSync(dir, { recursive: true }); | |
| } | |
| expect(fs.existsSync(dir)).toBe(false); | |
| await coordinator.save(); | |
| // save() should recreate the directory and state file | |
| expect(fs.existsSync(dir)).toBe(true); |
There was a problem hiding this comment.
Fixed in commit 849b756. Test now removes directory first with fs.rmSync(dir, { recursive: true }), then verifies save() recreates it.
scripts/article-quality-enhancer.js
Outdated
| import path from 'path'; | ||
| import { fileURLToPath } from 'url'; | ||
|
|
||
| const __filename = fileURLToPath(import.meta.url); | ||
| const __dirname = path.dirname(__filename); | ||
|
|
There was a problem hiding this comment.
path, fileURLToPath, __filename, and __dirname are declared but not used in this module. Removing unused imports/variables will avoid lint noise and keeps the script focused.
| import path from 'path'; | |
| import { fileURLToPath } from 'url'; | |
| const __filename = fileURLToPath(import.meta.url); | |
| const __dirname = path.dirname(__filename); |
There was a problem hiding this comment.
Fixed in commit 849b756. Removed unused imports (path, fileURLToPath, __filename, __dirname), kept only fs.
| grid.innerHTML = articlesToRender.map(article => { | ||
| // Generate language badge for the article | ||
| const flag = {en:'🇬🇧',sv:'🇸🇪',da:'🇩🇰',no:'🇳🇴',fi:'🇫🇮',de:'🇩🇪',fr:'🇫🇷',es:'🇪🇸',nl:'🇳🇱',ar:'🇸🇦',he:'🇮🇱',ja:'🇯🇵',ko:'🇰🇷',zh:'🇨🇳'}[article.lang] || '🌐'; | ||
| const langBadge = \`<span class="language-badge" aria-label="\${article.lang} language"><span aria-hidden="true">\${flag}</span> \${article.lang.toUpperCase()}</span>\`; | ||
|
|
||
| // Generate available languages display if multiple languages exist | ||
| const availableLangs = article.availableLanguages || [article.lang]; | ||
| let availableDisplay = ''; | ||
| if (availableLangs.length > 1) { | ||
| const availableText = '${AVAILABLE_IN_TRANSLATIONS[langKey] || 'Available in'}'; | ||
| const availableBadges = availableLangs.map(l => { | ||
| const f = {en:'🇬🇧',sv:'🇸🇪',da:'🇩🇰',no:'🇳🇴',fi:'🇫🇮',de:'🇩🇪',fr:'🇫🇷',es:'🇪🇸',nl:'🇳🇱',ar:'🇸🇦',he:'🇮🇱',ja:'🇯🇵',ko:'🇰🇷',zh:'🇨🇳'}[l] || '🌐'; | ||
| return \`<span class="lang-badge-sm"><span aria-hidden="true">\${f}</span> \${l.toUpperCase()}</span>\`; | ||
| }).join(' '); | ||
| availableDisplay = \`<p class="available-languages"><strong>\${availableText}:</strong> \${availableBadges}</p>\`; | ||
| } |
There was a problem hiding this comment.
The generated client-side rendering duplicates the language flag map and “available languages” HTML building logic instead of using the newly added LANGUAGE_FLAGS / generateLanguageBadge() / generateAvailableLanguages() helpers. This increases maintenance cost and risks the HTML/CSS drifting (e.g., lang-badge-sm vs language-badge, missing RTL handling, different aria-labels). Consider generating these badges via the shared helpers or emitting the needed data and reusing the same functions in one place.
There was a problem hiding this comment.
Fixed in commit 849b756. Eliminated duplication by emitting LANGUAGE_FLAGS and AVAILABLE_IN_TEXT to client-side:
const LANGUAGE_FLAGS = ${JSON.stringify(LANGUAGE_FLAGS)};
const AVAILABLE_IN_TEXT = '${escapeHtml(AVAILABLE_IN_TRANSLATIONS[langKey])}';Client code now uses these shared constants instead of inline flag maps.
scripts/generate-news-indexes.js
Outdated
| // Get ALL articles with language metadata for cross-language discovery | ||
| const allArticlesWithLanguageInfo = getAllArticlesWithLanguageInfo(articlesByLang); | ||
|
|
||
| const html = generateIndexHTML(langKey, allArticlesWithLanguageInfo, articlesByLang); |
There was a problem hiding this comment.
getAllArticlesWithLanguageInfo(articlesByLang) is recomputed inside the per-language loop, even though articlesByLang doesn’t change. Compute it once before the loop and reuse it for each generateIndexHTML() call to avoid repeated work on every run.
There was a problem hiding this comment.
Fixed in commit 849b756. Moved getAllArticlesWithLanguageInfo() outside the loop for 14× performance improvement:
const allArticlesWithLanguageInfo = getAllArticlesWithLanguageInfo(articlesByLang);
Object.keys(LANGUAGES).forEach(langKey => { ... });
|
@copilot apply changes based on the comments in this thread |
…irection, unused imports, and performance Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🔍 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 |
|
@copilot analyse all changes. why news/index.html contains 32 links to all languages. should only contain english links to news. Analyse all news/index_*.html and workflow and code or test changed. |
Analysis: Multi-Language Index BehaviorCurrent State ✅All 14 news/index_*.html files show ALL 32 articles (cross-language discovery)
Why This Was ImplementedThis behavior was explicitly requested in Issue #155: Original Problem Statement:
Desired State (from Issue #155):
Implementation DetailsFile: const allArticlesWithLanguageInfo = getAllArticlesWithLanguageInfo(articlesByLang);
Object.keys(LANGUAGES).forEach(langKey => {
const html = generateIndexHTML(langKey, allArticlesWithLanguageInfo, articlesByLang);
// ↑ All indexes get allArticlesWithLanguageInfo (32 articles)
});Result: Each article displays:
Alternative Behavior (Language-Specific Filtering)If you want language-specific indexes instead:
This would require reverting lines 947-954 to: Object.keys(LANGUAGES).forEach(langKey => {
const articles = articlesByLang[langKey] || [];
const html = generateIndexHTML(langKey, articles, articlesByLang);
});Question for ClarificationShould news indexes show:
The current implementation follows Issue #155's explicit requirements for cross-language article discovery to help readers find relevant content regardless of generation language. @pethers Please clarify which behavior you prefer, and I'll adjust accordingly. |
Analysis: Why All Index Files Show All 32 ArticlesCurrent BehaviorAll 14 Original Problem Statement (Issue #155):
Desired State (Issue #155):
Implementation DetailsFile: Line 409-449: function getAllArticlesWithLanguageInfo(articlesByLang) {
const allArticles = [];
// Collect ALL articles from ALL 14 languages
Object.keys(articlesByLang).forEach(lang => {
allArticles.push(...articlesByLang[lang]);
});
// Add availableLanguages field to each article
// Returns array of all 32 articles with language metadata
}Line 935-950: Index generation loop // Compute ONCE for all indexes
const allArticlesWithLanguageInfo = getAllArticlesWithLanguageInfo(articlesByLang);
// Pass SAME list to ALL 14 language indexes
Object.keys(LANGUAGES).forEach(langKey => {
const html = generateIndexHTML(langKey, allArticlesWithLanguageInfo, articlesByLang);
// Each index gets all 32 articles with language badges
});Current Distribution
If Requirement ChangedTo show only language-specific articles (news/index.html = EN only, news/index_fr.html = FR only): Revert to original behavior: // Line 935-950
Object.keys(LANGUAGES).forEach(langKey => {
const articlesForThisLang = articlesByLang[langKey] || [];
const html = generateIndexHTML(langKey, articlesForThisLang, articlesByLang);
// Each index shows only its language articles
});Result:
QuestionShould I revert the cross-language discovery feature and restore language-specific filtering? |
Each language index now displays only articles in that specific language: - news/index.html (EN): 10 English articles (was 32 all languages) - news/index_sv.html (SV): 10 Swedish articles (was 32 all languages) - news/index_fr.html (FR): 1 French article (was 32 all languages) Changed line 954 in generate-news-indexes.js from passing allArticlesWithLanguageInfo (all articles) to articlesByLang[langKey] (language-specific articles). This fixes the issue identified by @pethers where indexes were showing articles in all languages instead of just their target language. Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
|
Fixed in commit cd9e468. Analysis Results:
After Fix:
Each index now correctly shows only articles in its target language. Verified via Schema.org numberOfItems field and regenerated all 14 index files. |
CSS Architecture Refactor
Eliminated 660 lines of embedded
<style>tags from each generated article. All styles now instyles.cssunder.news-articlenamespace for better caching and maintainability.Added editorial tagline with proper
lang="en"attribute for non-English pages:Quality Framework
scripts/article-quality-enhancer.js- Economist-style quality validation:Score calculation weights: analytical depth 30%, party perspectives 25%, cross-references 25%, other 20%.
Workflow Coordination
scripts/workflow-state-coordinator.js- Deduplication and caching:Language Index Filtering
News indexes now show language-specific articles only:
Result:
news/index.htmlshows 10 EN articles,news/index_sv.htmlshows 10 SV articles, etc.Additional Fixes
dir="ltr"for Arabic/HebrewgetAllArticlesWithLanguageInfo()once before loopOriginal prompt
This section details on the original issue you should resolve
<issue_title>Improve news-realtime-monitor.md: Multi-language synchronization and quality enhancement</issue_title>
<issue_description>## 📋 Issue Type
Feature Enhancement / Bug Fix
🎯 Objective
Improve the news-realtime-monitor.md agentic workflow to ensure comprehensive multi-language synchronization, proper news index updates, and enhanced Economist-style quality standards for real-time Riksdag/Regering coverage.
📊 Current State
Workflow Analysis
File:
.github/workflows/news-realtime-monitor.mdSchedule: Twice daily (10:00 UTC, 14:00 UTC) on weekdays
Model: claude-opus-4.6
MCP Server: riksdag-regering (https://riksdag-regering-ai.onrender.com/mcp)
Timeout: 20 minutes
Current Issues Identified
Multi-Language Index Synchronization
Quality Framework Missing
Workflow Coordination Gap
Test Coverage
Measured Metrics
🚀 Desired State
Target Outcomes
Cross-Language Article Discovery
Economist-Style Quality Framework
Workflow State Coordination
Comprehensive Testing
🔧 Implementation Approach
Phase 1: Multi-Language Index Fix (Priority: HIGH)
Modify:
scripts/generate-news-indexes.jsKey Changes:
availableLanguagesdetection per article slugPhase 2: Quality Framework Implementation
Create:
scripts/article-quality-enhancer.jsFeatures:
Integration: