Add deep-inspection analysis: multi-stakeholder SWOT, economic dashboards, newsworthiness scoring#914
Conversation
… deep-inspection article type Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
… newsworthiness scoring Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
| buildEconomicTables, | ||
| } from '../scripts/data-transformers/content-generators/economic-dashboard-section.js'; | ||
| import type { EconomicDataPoint } from '../scripts/data-transformers/content-generators/economic-dashboard-section.js'; | ||
| import { INDICATOR_IDS, COUNTRY_CODES } from '../scripts/world-bank-client.js'; |
…ve readability 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 extends the news/article generation codebase with a new “deep-inspection” analysis capability by adding strategic newsworthiness scoring, multi-stakeholder SWOT section generation, and policy-domain-driven World Bank economic dashboard sections, plus workflow/config hooks to request targeted deep inspections.
Changes:
- Added new content generators:
newsworthiness,stakeholder-swot-section, andeconomic-dashboard-section(with barrel exports). - Extended CLI/config and workflow dispatch inputs to include a new
deep-inspectionarticle type and deep-inspection targeting inputs. - Added unit tests covering structure, i18n surface checks, and key behaviors for the new generators.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/stakeholder-swot-section.test.ts | Adds tests for multi-stakeholder SWOT section structure, escaping, and basic i18n coverage. |
| tests/newsworthiness.test.ts | Adds tests for multi-dimension scoring and deep-inspection threshold behavior (with a determinism gap). |
| tests/economic-dashboard-section.test.ts | Adds tests for indicator matching and dashboard/placeholder rendering behavior. |
| scripts/types/article.ts | Extends ArticleType union with 'deep-inspection'. |
| scripts/generate-news-indexes/helpers.ts | Classifies deep-inspection filenames as analysis content for indexes. |
| scripts/generate-news-enhanced/config.ts | Adds deep-inspection CLI args and allows 'deep-inspection' in VALID_ARTICLE_TYPES (but not wired end-to-end). |
| scripts/data-transformers/content-generators/stakeholder-swot-section.ts | Implements multi-stakeholder SWOT section generator with escaping and localized headings. |
| scripts/data-transformers/content-generators/newsworthiness.ts | Implements 6-dimension newsworthiness scoring and deep-inspection suggestion logic. |
| scripts/data-transformers/content-generators/index.ts | Re-exports the newly added content generators and types. |
| scripts/data-transformers/content-generators/economic-dashboard-section.ts | Implements economic indicator selection and dashboard/table generation (table headers are currently English-only). |
| .github/workflows/news-article-generator.md | Documents new workflow inputs for deep-inspection targeting (missing --document-urls in the “pass flags” rule). |
| .github/workflows/news-article-generator.lock.yml | Adds workflow_dispatch input descriptions for deep-inspection targeting fields. |
Comments suppressed due to low confidence (1)
scripts/generate-news-enhanced/config.ts:88
VALID_ARTICLE_TYPESnow includesdeep-inspection, but the currentgenerateNews()orchestrator doesn’t appear to handle this type (no case for it), so selecting it will be treated as an unknown type and nothing will be generated. Either implement adeep-inspectiongenerator path (and wire it into the switch) or avoid advertising/allowing this type until it’s supported end-to-end.
export const VALID_ARTICLE_TYPES: readonly string[] = ['week-ahead', 'month-ahead', 'weekly-review', 'monthly-review', 'committee-reports', 'propositions', 'motions', 'breaking', 'deep-inspection'];
export const articleTypes: string[] = typesArg
? (typesArg.split('=')[1] ?? '').split(',').map(t => t.trim())
: ['week-ahead'];
| /** Extract the value after '=' from a CLI argument, or empty string if absent. */ | ||
| function parseArgValue(arg: string | undefined): string { | ||
| return arg ? (arg.split('=')[1] ?? '').trim() : ''; | ||
| } |
There was a problem hiding this comment.
parseArgValue() uses arg.split('=')[1], which truncates values containing additional '=' (common in URLs with query strings). This can break --document-urls= and any future args that may include '='. Consider extracting everything after the first '=' (e.g., arg.slice(arg.indexOf('=') + 1)) instead of splitting and taking index 1.
| expect(score.overall).toBeGreaterThan(0); | ||
| // If high enough, should warrant deep inspection | ||
| if (score.overall >= 65) { | ||
| expect(score.warrantsDeepInspection).toBe(true); | ||
| expect(score.suggestedType).toBe('deep-inspection'); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
This test is conditional on score.overall >= 65, so it can pass even if the scoring logic never reaches the deep-inspection threshold. Make the assertion deterministic by crafting inputs that are guaranteed to exceed the threshold (and assert warrantsDeepInspection === true / suggestedType === 'deep-inspection'), and add an explicit boundary test around 64/65 to verify the threshold behavior.
| expect(score.overall).toBeGreaterThan(0); | |
| // If high enough, should warrant deep inspection | |
| if (score.overall >= 65) { | |
| expect(score.warrantsDeepInspection).toBe(true); | |
| expect(score.suggestedType).toBe('deep-inspection'); | |
| } | |
| }); | |
| expect(score.overall).toBeGreaterThanOrEqual(65); | |
| expect(score.warrantsDeepInspection).toBe(true); | |
| expect(score.suggestedType).toBe('deep-inspection'); | |
| }); | |
| it('applies deep inspection threshold around 65 points', () => { | |
| const lowScore = scoreNewsworthiness([makeLowDoc()]); | |
| expect(lowScore.overall).toBeLessThan(65); | |
| expect(lowScore.warrantsDeepInspection).toBe(false); | |
| const highScore = scoreNewsworthiness([makeStrategicDoc()], makeUnstableCIA()); | |
| expect(highScore.overall).toBeGreaterThanOrEqual(65); | |
| expect(highScore.warrantsDeepInspection).toBe(true); | |
| expect(highScore.suggestedType).toBe('deep-inspection'); | |
| }); |
| if (latestPoints.length > 0) { | ||
| tables.push({ | ||
| caption: `${indicator.name} (${latestYear}) — ${indicator.unit}`, | ||
| headers: ['Country', indicator.name, 'Unit'], | ||
| rows: latestPoints.map(p => [ | ||
| NORDIC_COMPARISON.countryNames[p.countryCode] ?? p.countryName, | ||
| p.value.toFixed(2), | ||
| indicator.unit, | ||
| ]), |
There was a problem hiding this comment.
buildEconomicTables() hard-codes English column headers ('Country', 'Unit'). When generating non-English articles, the accessibility table fallback will remain English, which undermines the module’s stated 14-language support. Consider localizing these headers (e.g., via L(lang, ...) with new label keys) or otherwise deriving translated header labels based on lang.
| 3. If **force_generation** is `true`, generate even if recent articles exist. | ||
| 4. If **languages** is empty/blank, default to `all` (14 languages). | ||
| 5. If **article_types** includes `deep-inspection`, use **document_ids**, **document_urls**, and **focus_topic** for targeted deep analysis. | ||
| 6. For `deep-inspection` type: pass `--document-ids` and `--focus-topic` flags to the generation script. |
There was a problem hiding this comment.
The deep-inspection workflow instructions mention using document_ids, document_urls, and focus_topic (Rule 5), but Rule 6 only mentions passing --document-ids and --focus-topic to the script. This leaves out --document-urls and may cause users to think URLs are unsupported even though config parsing was added. Update the instructions to include --document-urls (and keep flag formatting consistent with the script’s --flag=value parsing).
| 6. For `deep-inspection` type: pass `--document-ids` and `--focus-topic` flags to the generation script. | |
| 6. For `deep-inspection` type: pass `--document-ids=<value>`, `--document-urls=<value>`, and `--focus-topic=<value>` flags to the generation script. |
|
@copilot apply changes based on the comments in this thread |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
…stic threshold test, complete workflow docs Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
All 4 review comments addressed in
All 2739 tests passing, build succeeds. |
|
@copilot apply changes based on the comments in this thread |
- Replace hard-coded English "Policy Analysis" with per-language defaultTopicLabels map (14 languages) so no-focusTopic titles are fully localized (e.g. "Policyanalys" for sv, "Politikanalyse" for de) - Narrow document_urls workflow input description to "Riksdag documents" to match actual extractDocIdFromUrl() implementation scope - Updated lock.yml description to match Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Addressed in
Both review comments addressed in
All 2761 tests passing, CodeQL: 0 alerts. |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| export const articleTypes: string[] = rawArticleTypes.length > 0 | ||
| ? rawArticleTypes.filter(t => VALID_ARTICLE_TYPES.includes(t)) |
There was a problem hiding this comment.
articleTypes can become an empty array when --types is provided but all values are invalid (because invalid types are filtered out). In that case, generateNews() will do nothing without failing, which is easy to miss in workflows. Consider either (a) falling back to the default schedule/type when the filtered result is empty, or (b) exiting with a non-zero code / explicit error so the caller can detect misconfiguration.
| export const articleTypes: string[] = rawArticleTypes.length > 0 | |
| ? rawArticleTypes.filter(t => VALID_ARTICLE_TYPES.includes(t)) | |
| const filteredArticleTypes: string[] = rawArticleTypes.filter(t => VALID_ARTICLE_TYPES.includes(t)); | |
| if (rawArticleTypes.length > 0 && filteredArticleTypes.length === 0) { | |
| throw new Error( | |
| `No valid article types specified via --types. Valid types are: ${VALID_ARTICLE_TYPES.join(', ')}` | |
| ); | |
| } | |
| export const articleTypes: string[] = filteredArticleTypes.length > 0 | |
| ? filteredArticleTypes |
| const sanitizedTopic: string | null = focusTopic ? sanitizePlainText(focusTopic) : null; | ||
| const defaultTopicLabels: Record<Language, string> = { | ||
| en: 'Policy Analysis', | ||
| sv: 'Policyanalys', | ||
| da: 'Politisk analyse', | ||
| no: 'Politisk analyse', | ||
| fi: 'Politiikka-analyysi', |
There was a problem hiding this comment.
defaultTopicLabels is a constant map but it is re-created on every iteration of the for (const lang of languages) loop. Moving it (and potentially the titles template) outside the loop would reduce repeated allocations and keep the per-language loop focused on generation logic.
|
@copilot apply changes based on the comments in this thread |
…language loop Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Both review comments addressed in
All 2761 tests passing, CodeQL: 0 alerts. |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| const contentData = { reports: enrichedDocs as Parameters<typeof generateArticleContent>[0]['reports'] }; | ||
| const content: string = generateArticleContent(contentData, 'deep-inspection', lang); | ||
| const watchPoints = extractWatchPoints(contentData, lang); | ||
| const metadata = generateMetadata(contentData, 'deep-inspection', lang); | ||
| const readTime: string = calculateReadTime(content); |
There was a problem hiding this comment.
In generateDeepInspection(), contentData is built with reports: enrichedDocs, but generateArticleContent('deep-inspection', ...) falls through to generateGenericContent(), which reads from data.documents (not data.reports). This will produce the generic “no documents” placeholder instead of rendering the enriched documents. Populate contentData.documents (and adjust watchPoints/metadata inputs accordingly) so deep-inspection articles actually include the target documents.
| /** | ||
| * Strip HTML tags from a user-supplied string to prevent XSS. | ||
| * Uses defense-in-depth: first strips tag patterns in a loop to handle nested | ||
| * reconstruction (e.g. `<scr<script>ipt>`), then HTML-escapes any remaining | ||
| * special characters via escapeHtml() as a safety net. | ||
| */ | ||
| export function sanitizePlainText(text: string): string { | ||
| let cleaned = text; | ||
| let prev: string; | ||
| do { | ||
| prev = cleaned; | ||
| cleaned = cleaned.replace(/<[^>]*>/g, ''); | ||
| } while (cleaned !== prev); | ||
| return escapeHtml(cleaned); | ||
| } |
There was a problem hiding this comment.
sanitizePlainText() returns an HTML-escaped string via escapeHtml(). When that escaped string is later passed as title/subtitle into generateArticleHTML(), the template escapes title/subtitle again in meta tags and JSON-LD, resulting in double-escaped entities (e.g. & → &amp;) in structured data/SEO fields. Consider making sanitizePlainText() return plain text (tag-stripped) and fix escaping at render sites (ideally escape title/subtitle in the <title>, <h1>, and lede output), so escaping happens exactly once.
| export { generateStakeholderSwotSection } from './stakeholder-swot-section.js'; | ||
| export type { StakeholderSwotSectionOptions, StakeholderSwot } from './stakeholder-swot-section.js'; | ||
| export { generateEconomicDashboardSection, findIndicatorsForDomains, buildEconomicCharts, buildEconomicTables } from './economic-dashboard-section.js'; | ||
| export type { EconomicDashboardOptions, EconomicDataPoint } from './economic-dashboard-section.js'; | ||
| export { scoreNewsworthiness } from './newsworthiness.js'; | ||
| export type { NewsworthinessScore, NewsworthinessDimension } from './newsworthiness.js'; |
There was a problem hiding this comment.
This file exports the new generators/types, but the stable barrel (scripts/data-transformers/content-generators.ts, which is what scripts/data-transformers/index.ts re-exports) still only re-exports generateSwotSection/generateDashboardSection. As a result, consumers importing from ../data-transformers.js or ./content-generators.js won’t be able to access generateStakeholderSwotSection, generateEconomicDashboardSection, or scoreNewsworthiness. Add the corresponding exports/types to the stable barrel so the public API matches the documentation here.
|
@copilot apply changes based on the comments in this thread |
… double-escaping, and barrel exports Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
All 3 review comments addressed in
All 2761 tests passing, CodeQL: 0 alerts. |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| <meta name="description" content="${escapeHtml(subtitle).substring(0, 160)}"> | ||
| <meta name="keywords" content="${keywords.join(', ')}"> | ||
| <meta name="author" content="James Pether Sörling, CISSP, CISM"> |
There was a problem hiding this comment.
keywords are interpolated into the <meta name="keywords"> attribute without escaping. If any keyword contains ", <, or & (e.g., coming from document titles or user-provided topics in future), this can break the HTML or enable injection. Escape each keyword (or the joined string) before inserting into the attribute, consistent with how title, subtitle, and tags are handled.
contentDatato populatedocuments(notreports) sogenerateGenericContent()renders enriched docssanitizePlainText()double-escaping: returns plain text (tag-stripped only); escape happens at render sites (<title>,<h1>, lede) in template.tscontent-generators.tsanddata-transformers/index.ts)🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.