Conversation
- Updated MCPClient to handle tool name prefixes based on the server type (direct vs. gateway). - Added logic to skip prefixing for direct server requests and retry without prefix on tool not found errors. - Enhanced error handling for tool lookup failures. - Updated tests to verify correct behavior for both server types. - Removed mock MCP responses as they are no longer needed. - Added print styles and RTL support for news articles in CSS. - Improved article template tests to ensure correct HTML structure for different languages. - Added comprehensive tests for multi-language content labels and article content generation.
Contributor
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
Contributor
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the MCPClient to support both direct server connections and MCP gateway routing, improves multi-language article handling with comprehensive support for all 14 languages (including RTL for Arabic and Hebrew), and fixes party abbreviation detection edge cases. The changes align test expectations with actual implementation to prevent validation failures.
Changes:
- Refactored MCPClient with auto-detection for gateway vs direct server modes, using prefixed tool names for gateway and unprefixed for direct server
- Added comprehensive multi-language content labels (CONTENT_LABELS) covering all 14 supported languages with localized strings for article templates
- Implemented RTL support (Arabic & Hebrew) with dir="rtl" attribute and CSS overrides for proper text direction, borders, and layout
- Fixed party abbreviation detection regex to handle HTML tags and unified the pattern for all variants using Unicode-aware boundaries
- Externalized article CSS from embedded styles to styles.css link for better maintainability and consistency
- Updated test expectations to match actual tool usage in news-types modules (REQUIRED_TOOLS)
- Added detailed safe-outputs MCP tool usage instructions in workflow documentation for GitHub Agentic Workflows
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/mcp-client.js | Added gateway detection logic and skipPrefix parameter for fallback retry, improved error messages with tool lookup error detection |
| tests/mcp-client-core-part1.test.js | Updated tests to expect unprefixed tool names for direct server mode, added test for gateway mode with prefixed names |
| tests/mcp-client-core-part2.test.js | Updated all tool name assertions to expect unprefixed names for direct server URL |
| scripts/party-variants.js | Unified regex pattern for all party variants using Unicode-aware boundaries, removing special-case logic for single-letter codes |
| tests/party-variants.test.js | Added test cases for party abbreviations after HTML tags (MP, SD, KD) |
| scripts/data-transformers.js | Added CONTENT_LABELS object with translations for all 14 languages, implemented L() helper function for localized content generation |
| tests/data-transformers.test.js | Added comprehensive test suite for multi-language content labels and article generation |
| scripts/article-template.js | Added RTL detection and dir attribute, externalized CSS to styles.css link, removed embedded CSS generation |
| tests/article-template.test.js | Updated tests to verify external CSS link and RTL dir attribute for Arabic/Hebrew |
| styles.css | Added print styles and RTL overrides for news articles |
| tests/news-types/week-ahead.test.js | Aligned REQUIRED_TOOLS test to match actual implementation (single tool) |
| tests/news-types/committee-reports.test.js | Aligned REQUIRED_TOOLS test to match actual implementation (single tool) |
| tests/fixtures/mock-mcp-responses.json | Removed unused mock data file |
| package-lock.json | Updated dependency resolution (removed peer flags, added enquirer as optional) |
| .github/workflows/news-realtime-monitor.md | Added detailed safe-outputs MCP tool documentation and sandbox networking guidance |
| .github/workflows/news-evening-analysis.md | Added safe-outputs MCP tool usage instructions and MCP gateway configuration |
| .github/workflows/news-article-generator.md | Added MCP_SERVER_URL export and safe-outputs tool documentation |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor MCPClient to support direct server and gateway modes, enhancing error handling and tool lookup. Improve article templates with RTL support and print styles, while updating tests for multi-language content and tool requirements. Fix party abbreviation detection in the extractPartyMentions function.