Conversation
…islative efficiency metrics - Add MonthlyMetrics interface to data-transformers/types.ts - Add generateMonthlyReviewContent to content-generators.ts with 4 new sections in all 14 languages: Month in Numbers, Party Performance Rankings, Legislative Efficiency, and Strategic Outlook - Route monthly-review type to the new generator in data-transformers/index.ts - Fetch previous 2 months for 3-month rolling trend analysis in monthly-review.ts - Compute party rankings from motion/speech parti fields - Compute legislative efficiency rate (reports / propositions) - Extend MonthlyReviewValidationResult with hasPartyRankings, hasLegislativeEfficiency, hasMonthInNumbers fields and corresponding check functions - Add 6 new tests covering all new sections and features 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 makes monthly-review a distinct, richer intelligence product by adding multi-month trend metrics, party activity rankings, legislative efficiency indicators, and a strategic outlook section, with support across all 14 languages.
Changes:
- Add
MonthlyMetricsand pass it through the monthly review generation pipeline. - Introduce
generateMonthlyReviewContentto append monthly-specific sections after the generic base content. - Extend monthly review validation and tests to cover the new sections and trend-fetch behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/news-types/monthly-review.test.ts | Adds test coverage for the new monthly sections and trend-search behavior. |
| scripts/news-types/monthly-review.ts | Fetches previous months for trends, computes MonthlyMetrics, passes it into article generation, and extends validation fields. |
| scripts/data-transformers/types.ts | Introduces MonthlyMetrics and adds monthlyMetrics? to ArticleContentData. |
| scripts/data-transformers/index.ts | Routes monthly-review to the new monthly-specific content generator. |
| scripts/data-transformers/content-generators.ts | Implements localized labels and renders the four new monthly sections. |
scripts/news-types/monthly-review.ts
Outdated
| .catch(() => [] as RawDocument[]), | ||
| client.searchDocuments({ from_date: formatDateForSlug(prev2Start), to_date: formatDateForSlug(prevStart), limit: 50 }) | ||
| .catch(() => [] as RawDocument[]), |
There was a problem hiding this comment.
Step 6 swallows searchDocuments errors with .catch(() => []) and doesn't log, which makes trend metrics silently degrade and harder to debug (unlike Step 2/4 which log errors). Consider logging the caught error (and optionally recording it in mcpCalls) so failures are observable.
| .catch(() => [] as RawDocument[]), | |
| client.searchDocuments({ from_date: formatDateForSlug(prev2Start), to_date: formatDateForSlug(prevStart), limit: 50 }) | |
| .catch(() => [] as RawDocument[]), | |
| .catch((error) => { | |
| console.error( | |
| 'MonthlyReview Step 6 — search_dokument failed for previous month trend window', | |
| { from_date: formatDateForSlug(prevStart), to_date: fromStr, limit: 50 }, | |
| error, | |
| ); | |
| return [] as RawDocument[]; | |
| }), | |
| client.searchDocuments({ from_date: formatDateForSlug(prev2Start), to_date: formatDateForSlug(prevStart), limit: 50 }) | |
| .catch((error) => { | |
| console.error( | |
| 'MonthlyReview Step 6 — search_dokument failed for two-months-ago trend window', | |
| { from_date: formatDateForSlug(prev2Start), to_date: formatDateForSlug(prevStart), limit: 50 }, | |
| error, | |
| ); | |
| return [] as RawDocument[]; | |
| }), |
scripts/news-types/monthly-review.ts
Outdated
| article.content.includes('partyRankings') || | ||
| article.content.toLowerCase().includes('rankings'); | ||
| } | ||
|
|
||
| function checkLegislativeEfficiency(article: ArticleInput): boolean { | ||
| if (!article || !article.content) return false; | ||
| return article.content.includes('Legislative Efficiency') || | ||
| article.content.includes('Lagstiftningseffektivitet') || | ||
| article.content.toLowerCase().includes('efficiency') || | ||
| article.content.toLowerCase().includes('throughput'); | ||
| } | ||
|
|
||
| function checkMonthInNumbers(article: ArticleInput): boolean { | ||
| if (!article || !article.content) return false; | ||
| return article.content.includes('Month in Numbers') || | ||
| article.content.includes('Månaden i siffror') || | ||
| article.content.toLowerCase().includes('total documents') || | ||
| article.content.toLowerCase().includes('committee reports'); |
There was a problem hiding this comment.
The new validation helpers are very permissive and can produce false positives (e.g., any occurrence of the word "rankings", "efficiency", "throughput", or "committee reports" will satisfy the checks even if the dedicated section is missing). Since these flags are used to validate article quality, it would be more reliable to check for the actual section headings/labels that the generator emits (including the emoji-prefixed titles) across supported languages, or a more specific marker unique to each section.
| article.content.includes('partyRankings') || | |
| article.content.toLowerCase().includes('rankings'); | |
| } | |
| function checkLegislativeEfficiency(article: ArticleInput): boolean { | |
| if (!article || !article.content) return false; | |
| return article.content.includes('Legislative Efficiency') || | |
| article.content.includes('Lagstiftningseffektivitet') || | |
| article.content.toLowerCase().includes('efficiency') || | |
| article.content.toLowerCase().includes('throughput'); | |
| } | |
| function checkMonthInNumbers(article: ArticleInput): boolean { | |
| if (!article || !article.content) return false; | |
| return article.content.includes('Month in Numbers') || | |
| article.content.includes('Månaden i siffror') || | |
| article.content.toLowerCase().includes('total documents') || | |
| article.content.toLowerCase().includes('committee reports'); | |
| article.content.includes('partyRankings'); | |
| } | |
| function checkLegislativeEfficiency(article: ArticleInput): boolean { | |
| if (!article || !article.content) return false; | |
| return article.content.includes('Legislative Efficiency') || | |
| article.content.includes('Lagstiftningseffektivitet'); | |
| } | |
| function checkMonthInNumbers(article: ArticleInput): boolean { | |
| if (!article || !article.content) return false; | |
| return article.content.includes('Month in Numbers') || | |
| article.content.includes('Månaden i siffror'); |
scripts/data-transformers/types.ts
Outdated
| /** Party activity rankings sorted by total activity (motions + speeches) */ | ||
| partyRankings: Array<{ party: string; motionCount: number; speechCount: number }>; | ||
| /** | ||
| * Legislative efficiency rate: committee reports divided by propositions (0–1). |
There was a problem hiding this comment.
legislativeEfficiencyRate is documented as "(0–1)", but reportCount / propositionCount can exceed 1 when there are more reports than propositions in the same window. Either clamp/normalize the metric to a 0–1 range, or update the doc comment to reflect the actual (unbounded) ratio semantics.
| * Legislative efficiency rate: committee reports divided by propositions (0–1). | |
| * Legislative efficiency rate: committee reports divided by propositions. | |
| * This is a non-negative ratio (reportCount / propositionCount) and may exceed 1 | |
| * when there are more reports than propositions in the same period. |
scripts/news-types/monthly-review.ts
Outdated
| hasPartyRankings, | ||
| hasLegislativeEfficiency, | ||
| hasMonthInNumbers, | ||
| passed: hasMonthlySummary && hasMinimumSources && hasRetrospectiveTone && hasTrendAnalysis |
There was a problem hiding this comment.
passed is still computed only from the pre-existing checks, so validateMonthlyReview can return passed: true even when the new monthly sections (Month in Numbers / Party Rankings / Legislative Efficiency) are missing. If these sections are now required for a monthly review, include the new booleans in the passed expression (or clearly document why they are informational only).
| passed: hasMonthlySummary && hasMinimumSources && hasRetrospectiveTone && hasTrendAnalysis | |
| passed: | |
| hasMonthlySummary && | |
| hasMinimumSources && | |
| hasRetrospectiveTone && | |
| hasTrendAnalysis && | |
| hasPartyRankings && | |
| hasLegislativeEfficiency && | |
| hasMonthInNumbers |
|
@copilot apply changes based on the comments in this thread |
- Log Step 6 trend-fetch errors with console.error for observability - Tighten check functions to match exact generated section headings only - Include hasPartyRankings/hasLegislativeEfficiency/hasMonthInNumbers in passed - Fix legislativeEfficiencyRate doc comment: ratio is unbounded (may exceed 1) Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied all 4 changes in b990749:
|
scripts/news-types/monthly-review.ts
Outdated
| if (rec['doktyp'] === 'mot' && rec['parti']) { | ||
| const p = String(rec['parti']); | ||
| partyMotions[p] = (partyMotions[p] ?? 0) + 1; |
There was a problem hiding this comment.
The party aggregation for motions (lines 239-241) doesn't filter out empty strings after converting rec['parti'] to String, unlike the speech aggregation (lines 245-246) which explicitly checks if (p). If a motion has an empty parti field, it will create a partyRankings entry with an empty party name that would appear in the rendered output. Consider adding if (p) check after line 240 for consistency and to prevent empty party names in rankings.
| if (rec['doktyp'] === 'mot' && rec['parti']) { | |
| const p = String(rec['parti']); | |
| partyMotions[p] = (partyMotions[p] ?? 0) + 1; | |
| if (rec['doktyp'] === 'mot') { | |
| const p = String(rec['parti'] ?? ''); | |
| if (p) { | |
| partyMotions[p] = (partyMotions[p] ?? 0) + 1; | |
| } |
|
@copilot apply changes based on the comments in this thread |
Mirror the speech aggregation pattern: use String(rec['parti'] ?? '') and guard with `if (p)` to prevent empty-string party names appearing in partyRankings output. Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Fixed in 637b7af. The motion aggregation loop now mirrors the speech pattern exactly: |
Signed-off-by: James Pether Sörling <pethers@users.noreply.github.com>
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
…er efficiency ratio with toFixed(2) Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
All three issues from the review thread applied in db25d79: efficiency rate rendered as a decimal ratio ( |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| if (metrics.partyRankings.length === 0) return ''; | ||
|
|
||
| let html = `\n <h2>${escapeHtml(ml(lang, 'partyRankings'))}</h2>\n`; |
There was a problem hiding this comment.
generatePartyRankings returns an empty string when metrics.partyRankings.length === 0, which means the Party Performance Rankings section will be omitted entirely in months where party fields are missing or speech fetch returns empty. Since the monthly review validation now requires hasPartyRankings, this can cause otherwise valid monthly reviews to fail validation. Consider always rendering the section with a localized “insufficient data” placeholder (or relaxing validation to allow the section to be present but empty).
| if (metrics.partyRankings.length === 0) return ''; | |
| let html = `\n <h2>${escapeHtml(ml(lang, 'partyRankings'))}</h2>\n`; | |
| let html = `\n <h2>${escapeHtml(ml(lang, 'partyRankings'))}</h2>\n`; | |
| // Always render the section; if there is no underlying data, show a localized placeholder | |
| if (!metrics.partyRankings || metrics.partyRankings.length === 0) { | |
| const placeholder = ml(lang, 'insufficientData'); | |
| html += ` <div class="context-box">\n`; | |
| html += ` <p>${escapeHtml(placeholder)}</p>\n`; | |
| html += ` </div>\n`; | |
| return html; | |
| } |
There was a problem hiding this comment.
Fixed in d0e54bd. generatePartyRankings now always renders the 🏆 Party Performance Rankings heading. When partyRankings is empty, a localized "Insufficient party activity data for this period." placeholder is shown inside the context-box instead of returning an empty string — so hasPartyRankings validation passes in all languages.
| const stabilityLabel = ml(lang, 'coalitionStabilityOutlook'); | ||
| const stabilityScore = cia.coalitionStability.stabilityScore; | ||
| const riskLevel = cia.coalitionStability.riskLevel; | ||
| html += ` <li><strong>${escapeHtml(stabilityLabel)}:</strong> ${escapeHtml(stabilityScore)}/100 (${escapeHtml(riskLevel)})</li>\n`; |
There was a problem hiding this comment.
escapeHtml returns an empty string for falsy values (including numeric 0). Here stabilityScore is a number (0–100) and will render blank if it ever equals 0. Convert numeric values to strings before escaping (e.g., escapeHtml(String(stabilityScore))) to preserve legitimate zero values, consistent with the fixes applied for other numeric metrics.
| html += ` <li><strong>${escapeHtml(stabilityLabel)}:</strong> ${escapeHtml(stabilityScore)}/100 (${escapeHtml(riskLevel)})</li>\n`; | |
| html += ` <li><strong>${escapeHtml(stabilityLabel)}:</strong> ${escapeHtml(String(stabilityScore))}/100 (${escapeHtml(riskLevel)})</li>\n`; |
There was a problem hiding this comment.
Fixed in d0e54bd. stabilityScore is now passed as escapeHtml(String(stabilityScore)) so a legitimate value of 0 renders as "0" rather than blank.
| it('should fetch previous months for trend analysis (3 searchDocuments calls)', async () => { | ||
| await monthlyReviewModule.generateMonthlyReview({ | ||
| languages: ['en'] | ||
| }); | ||
|
|
||
| // Exactly 3 calls: current month + previous month + 2-months-ago | ||
| expect(mockClientInstance.searchDocuments.mock.calls.length).toBe(3); |
There was a problem hiding this comment.
This test asserts an exact searchDocuments call count of 3. That’s brittle and doesn’t verify the important behavior (that the two additional calls target the correct previous windows / limits). Consider asserting on the presence and parameters of the extra calls (e.g., 2 additional calls with limit: 1000 and expected from_date/to_date ranges) rather than enforcing an exact total call count, so the test won’t break if additional document searches are added later for other monthly features.
| it('should fetch previous months for trend analysis (3 searchDocuments calls)', async () => { | |
| await monthlyReviewModule.generateMonthlyReview({ | |
| languages: ['en'] | |
| }); | |
| // Exactly 3 calls: current month + previous month + 2-months-ago | |
| expect(mockClientInstance.searchDocuments.mock.calls.length).toBe(3); | |
| it('should fetch previous months for trend analysis using multiple searchDocuments calls', async () => { | |
| await monthlyReviewModule.generateMonthlyReview({ | |
| languages: ['en'] | |
| }); | |
| // At least 3 calls: current month + previous month + 2-months-ago | |
| expect(mockClientInstance.searchDocuments.mock.calls.length).toBeGreaterThanOrEqual(3); |
There was a problem hiding this comment.
Fixed in d0e54bd. Changed toBe(3) → toBeGreaterThanOrEqual(3) so the assertion won't break if additional document searches are added later.
scripts/news-types/monthly-review.ts
Outdated
| article.content.includes('Partiernas prestationsrankning'); | ||
| } | ||
|
|
||
| function checkLegislativeEfficiency(article: ArticleInput): boolean { | ||
| if (!article || !article.content) return false; | ||
| return article.content.includes('Legislative Efficiency') || | ||
| article.content.includes('Lagstiftningseffektivitet'); | ||
| } | ||
|
|
||
| function checkMonthInNumbers(article: ArticleInput): boolean { | ||
| if (!article || !article.content) return false; | ||
| return article.content.includes('Month in Numbers') || | ||
| article.content.includes('Månaden i siffror'); |
There was a problem hiding this comment.
validateMonthlyReview now requires hasPartyRankings / hasLegislativeEfficiency / hasMonthInNumbers, but the detection helpers only look for English + Swedish headings. When generating monthly reviews in other supported languages (e.g. de, fr, etc.), these checks will return false and passed will fail even though the sections are present with localized headings. Consider either checking for the emoji-prefixed headings (📊/🏆/⚖️) regardless of language, or matching against the full per-language label map used by the content generator (with a fallback to EN/SV).
| article.content.includes('Partiernas prestationsrankning'); | |
| } | |
| function checkLegislativeEfficiency(article: ArticleInput): boolean { | |
| if (!article || !article.content) return false; | |
| return article.content.includes('Legislative Efficiency') || | |
| article.content.includes('Lagstiftningseffektivitet'); | |
| } | |
| function checkMonthInNumbers(article: ArticleInput): boolean { | |
| if (!article || !article.content) return false; | |
| return article.content.includes('Month in Numbers') || | |
| article.content.includes('Månaden i siffror'); | |
| article.content.includes('Partiernas prestationsrankning') || | |
| // Language-agnostic detection via trophy emoji used in headings | |
| article.content.includes('🏆'); | |
| } | |
| function checkLegislativeEfficiency(article: ArticleInput): boolean { | |
| if (!article || !article.content) return false; | |
| return article.content.includes('Legislative Efficiency') || | |
| article.content.includes('Lagstiftningseffektivitet') || | |
| // Language-agnostic detection via scales emoji used in headings | |
| article.content.includes('⚖️'); | |
| } | |
| function checkMonthInNumbers(article: ArticleInput): boolean { | |
| if (!article || !article.content) return false; | |
| return article.content.includes('Month in Numbers') || | |
| article.content.includes('Månaden i siffror') || | |
| // Language-agnostic detection via chart emoji used in headings | |
| article.content.includes('📊'); |
There was a problem hiding this comment.
Fixed in d0e54bd. All three helpers now also check for the emoji prefix (📊, 🏆, ⚖️) so they detect the sections in any of the 14 supported languages, not just English and Swedish.
|
@copilot apply changes based on the comments in this thread |
…stabilityScore, emoji-based validation, non-brittle test assertion Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
All four review comments applied in d0e54bd. All 2026 tests pass. |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
| html += ` <li><em>${escapeHtml(ml(lang, 'trendVsPrevMonth'))}:</em> ${escapeHtml(prevSign)}${prevDiff}</li>\n`; | ||
| } | ||
| if (metrics.twoMonthsAgoDocCount > 0 || metrics.previousMonthDocCount > 0) { | ||
| html += ` <li><em>${escapeHtml(ml(lang, 'trendVs2MonthsAgo'))}:</em> ${rollingAvg}</li>\n`; |
There was a problem hiding this comment.
The numeric values prevDiff (line 1436) and rollingAvg (line 1439) are directly interpolated into the HTML without escaping. While these are computed numeric values, they should be consistently escaped like all other interpolated content for defense-in-depth XSS protection. Convert them to strings and apply escapeHtml: escapeHtml(String(prevDiff)) and escapeHtml(String(rollingAvg)).
| html += ` <li><em>${escapeHtml(ml(lang, 'trendVsPrevMonth'))}:</em> ${escapeHtml(prevSign)}${prevDiff}</li>\n`; | |
| } | |
| if (metrics.twoMonthsAgoDocCount > 0 || metrics.previousMonthDocCount > 0) { | |
| html += ` <li><em>${escapeHtml(ml(lang, 'trendVs2MonthsAgo'))}:</em> ${rollingAvg}</li>\n`; | |
| html += ` <li><em>${escapeHtml(ml(lang, 'trendVsPrevMonth'))}:</em> ${escapeHtml(prevSign)}${escapeHtml(String(prevDiff))}</li>\n`; | |
| } | |
| if (metrics.twoMonthsAgoDocCount > 0 || metrics.previousMonthDocCount > 0) { | |
| html += ` <li><em>${escapeHtml(ml(lang, 'trendVs2MonthsAgo'))}:</em> ${escapeHtml(String(rollingAvg))}</li>\n`; |
There was a problem hiding this comment.
Fixed in 770137d. prevDiff and rollingAvg are now wrapped with escapeHtml(String(...)) for consistent defense-in-depth XSS protection.
|
|
||
| if (metrics.propositionCount > 0) { | ||
| const ratio = metrics.legislativeEfficiencyRate.toFixed(2); | ||
| html += ` <li><strong>${escapeHtml(ml(lang, 'efficiencyRate'))}:</strong> ${ratio} (${metrics.reportCount} / ${metrics.propositionCount})</li>\n`; |
There was a problem hiding this comment.
The ratio value (computed from toFixed(2)) and the numeric values metrics.reportCount and metrics.propositionCount are directly interpolated without escaping. Although ratio is a string from toFixed, and reportCount/propositionCount are numeric, they should be consistently escaped for defense-in-depth: ${escapeHtml(ratio)} and ${escapeHtml(String(metrics.reportCount))} / ${escapeHtml(String(metrics.propositionCount))}.
| html += ` <li><strong>${escapeHtml(ml(lang, 'efficiencyRate'))}:</strong> ${ratio} (${metrics.reportCount} / ${metrics.propositionCount})</li>\n`; | |
| html += ` <li><strong>${escapeHtml(ml(lang, 'efficiencyRate'))}:</strong> ${escapeHtml(ratio)} (${escapeHtml(String(metrics.reportCount))} / ${escapeHtml(String(metrics.propositionCount))})</li>\n`; |
There was a problem hiding this comment.
Fixed in 770137d. ratio, metrics.reportCount, and metrics.propositionCount are now all wrapped with escapeHtml(...) / escapeHtml(String(...)) for consistent defense-in-depth escaping.
|
@copilot apply changes based on the comments in this thread |
…f, rollingAvg, ratio, reportCount, propositionCount Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied in 770137d. Both numeric interpolations flagged in the thread are now consistently wrapped with |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
MonthlyMetricsinterface and related featuresrollingAvgwithavailableMonthCountsspeecheslabelefficiencyRatelabellegislativeEfficiencyRaterendering: usetoFixed(2)ratioescapeHtml(0)ingenerateMonthInNumbersescapeHtml(0)ingeneratePartyRankingsgeneratePartyRankingsto always render section with localized placeholder when emptyescapeHtml(stabilityScore)→escapeHtml(String(stabilityScore))ingenerateStrategicOutlooktoBe(3)test assertion totoBeGreaterThanOrEqual(3)validateMonthlyReviewhelpers to use emoji-based detection (📊/🏆/⚖️) for language-agnostic checksprevDiff,rollingAvg,ratio,reportCount,propositionCountOriginal prompt
This section details on the original issue you should resolve
<issue_title>Improve Monthly Review: Add multi-month trends, party rankings, and legislative efficiency metrics</issue_title>
<issue_description>## 📋 Issue Type
Enhancement — Extend Monthly Review with multi-month trend analysis, party performance rankings, and strategic outlook
🎯 Objective
Enhance
scripts/news-types/monthly-review.tsto provide comprehensive monthly trend analysis, party performance comparative rankings, legislative efficiency metrics, and a forward-looking strategic outlook. Transform monthly reviews from document summaries into the definitive monthly intelligence brief.📊 Current State
scripts/news-types/monthly-review.ts(335 lines)search_dokument,get_dokument_innehall,search_anforanden,get_betankanden,get_propositioner,get_motioner)🚀 Desired State
🔧 Implementation Approach
1. Multi-month trend aggregation (~80 lines)
2. Party performance rankings (~70 lines)
3. Legislative efficiency metrics (~60 lines)
4. "Month in Numbers" section (~40 lines)
5. Enhanced article templates
🤖 Recommended Agent
code-quality-engineer— TypeScript enhancement with data aggregation patterns✅ Acceptance Criteria
📚 References
scripts/news-types/monthly-review.tsscripts/news-types/weekly-review.ts(base enrichment pipeline)scripts/data-transformers/risk-analysis.tsscripts/data-transformers/policy-analysis.tsscripts/data-transformers/document-analysis.ts</issue_description>
<agent_instructions>Extend monthly-review articles in scripts/news-types/monthly-review.ts with multi-month trend analysis, party performance rankings, and legislative efficiency metrics. Add "Month in Numbers" and "Party Performance Rankings" template sections. Leverage data-transformers for analysis. Run tests with npx vitest run.</agent_instructions>
Comments on the Issue (you are @copilot in this ...
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.