Integrate World Bank MCP server for economic context in news generation#653
Integrate World Bank MCP server for economic context in news generation#653
Conversation
- Add world-bank MCP server to .github/copilot-mcp.json - Create scripts/world-bank-client.ts (TypeScript REST client for World Bank API) - Create scripts/world-bank-context.ts (economic context provider with Swedish indicators) - Integrate economic context detection into article-quality-enhancer.ts - Update news-evening-analysis.md workflow with World Bank data instructions - Update news-article-generator.md workflow with World Bank reference - Add tests/world-bank-client.test.ts (30 tests) - Add tests/world-bank-context.test.ts (35 tests) - All 2102 tests pass Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🏷️ Automatic Labeling SummaryThis PR has been automatically labeled based on the files changed and PR metadata. Applied Labels: documentation,workflow,riksdag-data,ci-cd,testing,refactor,size-xl,news,agentic-workflow Label Categories
For more information, see |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
…egex comment 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
Integrates World Bank economic indicator support into the news-generation toolchain to optionally enrich articles with macroeconomic context and to surface a non-blocking quality recommendation for including that context.
Changes:
- Added a World Bank REST client and an economic-context mapping/provider (indicators → policy areas/committees + localized headings).
- Extended article quality scoring inputs with an optional
hasEconomicContextmetric and a non-blocking “recommend economic context” warning. - Updated agentic workflow docs and added Vitest coverage for the new client/context modules.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/world-bank-client.ts |
New World Bank API client with retry/timeout, indicator/country constants, and helper methods. |
scripts/world-bank-context.ts |
New indicator→policy/committee mapping, localized section headings, and economic-context detection helpers. |
scripts/article-quality-enhancer.ts |
Adds hasEconomicContext metric collection and a non-blocking recommendation warning. |
scripts/types/validation.ts |
Extends QualityThresholds/QualityMetrics with optional economic-context fields. |
tests/world-bank-client.test.ts |
Unit tests for client behavior (fetching, retry, error handling, compareCountries, constants). |
tests/world-bank-context.test.ts |
Unit tests for mappings, headings, indicator lookup, and context detection. |
.github/workflows/news-evening-analysis.md |
Documents optional World Bank MCP usage and indicator references for evening analysis. |
.github/workflows/news-article-generator.md |
Documents optional World Bank MCP usage and indicator references for article generation. |
.github/copilot-mcp.json |
Adds world-bank MCP server configuration via @smithery/cli. |
| return (data[1] as RawIndicatorValue[]) | ||
| .filter((item): item is RawIndicatorValue & { value: number } => item.value !== null && item.value !== undefined) | ||
| .map((item) => ({ | ||
| countryId: item.country?.id ?? countryCode, | ||
| countryName: item.country?.value ?? countryCode, |
There was a problem hiding this comment.
getIndicator()’s JSDoc says results are sorted by date descending, but the implementation currently returns the API order without sorting. Since getLatestIndicator() and compareCountries() assume index 0 is the most recent, please sort the mapped results by numeric year (descending) (and optionally push empty/invalid dates to the end) before returning.
There was a problem hiding this comment.
Fixed in a75983e. getIndicator() now sorts mapped results by numeric year descending, with invalid/empty dates pushed to the end. Added a dedicated test should sort results by date descending that verifies out-of-order API responses are correctly reordered.
scripts/world-bank-client.ts
Outdated
| readonly indicatorId: string; | ||
| readonly indicatorName: string; | ||
| readonly date: string; | ||
| readonly value: number | null; |
There was a problem hiding this comment.
WorldBankDataPoint.value is typed as number | null, but getIndicator() filters out all null/undefined values, so callers will never see a data point with value: null. To avoid confusion for API consumers, either (a) change the type to number for returned data points (and keep filtering), or (b) stop filtering and let value be null when the API reports missing data (and update getLatestIndicator() accordingly).
| readonly value: number | null; | |
| readonly value: number; |
There was a problem hiding this comment.
Fixed in a75983e. Changed value type from number | null to number since the filter already excludes all null/undefined entries before returning.
scripts/world-bank-context.ts
Outdated
| const lowerQuery = query.toLowerCase(); | ||
| return ECONOMIC_INDICATORS.filter( | ||
| (indicator) => | ||
| indicator.policyAreas.some((area) => area.toLowerCase().includes(lowerQuery)) || | ||
| indicator.committees.some((c) => c.toLowerCase() === lowerQuery), |
There was a problem hiding this comment.
findRelevantIndicators() lowercases the query but doesn’t trim or guard against an empty/whitespace query. Because "" is contained in every string, an empty query will match every indicator via includes(''). Consider using const q = query.trim().toLowerCase(); and returning [] early when q.length === 0.
| const lowerQuery = query.toLowerCase(); | |
| return ECONOMIC_INDICATORS.filter( | |
| (indicator) => | |
| indicator.policyAreas.some((area) => area.toLowerCase().includes(lowerQuery)) || | |
| indicator.committees.some((c) => c.toLowerCase() === lowerQuery), | |
| const q = query.trim().toLowerCase(); | |
| if (q.length === 0) { | |
| return []; | |
| } | |
| return ECONOMIC_INDICATORS.filter( | |
| (indicator) => | |
| indicator.policyAreas.some((area) => area.toLowerCase().includes(q)) || | |
| indicator.committees.some((c) => c.toLowerCase() === q), |
There was a problem hiding this comment.
Fixed in a75983e. findRelevantIndicators() now trims and checks for empty query, returning [] early when q.length === 0. Added tests for both empty string and whitespace-only inputs.
scripts/world-bank-context.ts
Outdated
| /\bförsvarsutgifter\b/i, // Swedish: defense expenditure | ||
| /\bforskningsutgifter\b/i, // Swedish: R&D expenditure |
There was a problem hiding this comment.
In hasEconomicContext(), patterns like \bförsvarsutgifter\b / \bforskningsutgifter\b will miss common inflected forms (e.g. försvarsutgifterna, forskningsutgifterna) due to the trailing word-boundary. Consider matching on the stem (e.g. försvarsutgift / forskningsutgift) or relaxing the end boundary so plural/definite forms are detected too.
| /\bförsvarsutgifter\b/i, // Swedish: defense expenditure | |
| /\bforskningsutgifter\b/i, // Swedish: R&D expenditure | |
| /\bförsvarsutgift/i, // Swedish: defense expenditure (försvarsutgift, försvarsutgifter, försvarsutgifterna, etc.) | |
| /\bforskningsutgift/i, // Swedish: R&D expenditure (forskningsutgift, forskningsutgifter, forskningsutgifterna, etc.) |
There was a problem hiding this comment.
Fixed in a75983e. Changed /\bförsvarsutgifter\b/i → /\bförsvarsutgift/i and /\bforskningsutgifter\b/i → /\bforskningsutgift/i to match the stem, covering definite/plural forms (försvarsutgifterna, forskningsutgifterna, etc.). Added a test should detect Swedish inflected forms (definite, plural) for these patterns.
| "-y", | ||
| "@smithery/cli@latest", | ||
| "run", | ||
| "@anshumax/world_bank_mcp_server" | ||
| ], |
There was a problem hiding this comment.
The new MCP server config uses npx -y @smithery/cli@latest run @anshumax/world_bank_mcp_server which pulls whatever versions are current at runtime. For reproducibility and supply-chain risk reduction, consider pinning both packages to explicit versions (and updating them deliberately via dependency bumps).
There was a problem hiding this comment.
Fixed in a75983e. Pinned @smithery/cli to 4.4.0. Note: @anshumax/world_bank_mcp_server is a Python package (v0.1.0) hosted on Smithery's registry — it doesn't support npm-style version pinning in the run command. The @smithery/cli version pin controls the client-side tooling.
|
@copilot apply changes based on the comments in this thread |
…wedish inflections, pin versions - Sort getIndicator() results by date descending (most recent first) - Change WorldBankDataPoint.value type from `number | null` to `number` - Guard findRelevantIndicators() against empty/whitespace queries - Fix Swedish word patterns to match inflected forms (försvarsutgifterna, etc.) - Pin @smithery/cli to v4.4.0 for supply-chain safety - Add 4 new tests covering sort, empty query, inflected forms Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
All 5 review comments addressed in a75983e:
4 new tests added, 2106 total passing. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
.github/copilot-mcp.json(pinned@smithery/cli@4.4.0)scripts/world-bank-client.ts— REST client with date-descending sortscripts/world-bank-context.ts— Economic context with empty-query guard and Swedish inflection supportscripts/article-quality-enhancer.tsgetIndicator()results by date descendingWorldBankDataPoint.valuetype tonumber(nulls already filtered)findRelevantIndicators()against empty/whitespace queries@smithery/clito4.4.0for reproducibility✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.