Skip to content

feat: decompose chrome.ts into bounded-context sub-modules with comprehensive tests#2285

Merged
pethers merged 6 commits intomainfrom
copilot/improve-html-rendering-pipeline
May 6, 2026
Merged

feat: decompose chrome.ts into bounded-context sub-modules with comprehensive tests#2285
pethers merged 6 commits intomainfrom
copilot/improve-html-rendering-pipeline

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 6, 2026

  • Decompose chrome.ts (742 lines) into focused sub-modules (head, header, footer, helpers, types) each ≤200 lines
  • Fix the single any type in chrome.ts (replaced with typed JsonLdNode interface)
  • Add 59 comprehensive tests for chrome components (HTML5 structure, RTL, accessibility, JSON-LD, hreflang, SEO)
  • Add JSON-LD Schema.org NewsArticle/BreadcrumbList/Speakable validation tests (18 tests)
  • Add markdown pipeline independence tests and security boundary tests (15 tests)
  • Remove unused Language/BASE_URL imports from header.ts and footer.ts
  • Replace generate-sitemap-html.js (CLI side-effects) with sitemap-html/index.js in all chrome sub-modules
  • Fix jsonLd docstring: clarify plain objects required (not pre-serialised strings)
  • Align RSS href default in head with footer: per-language /rss_{lang}.xml
  • Fix unused SiteChrome import in chrome-decomposition.test.ts (caused TS6196 type-check failure)
  • All 59 chrome tests pass, TypeScript type-check clean

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🏷️ Automatic Labeling Summary

This PR has been automatically labeled based on the files changed and PR metadata.

Applied Labels: size-xs

Label Categories

  • 🗳️ Content: news, dashboard, visualization, intelligence
  • 💻 Technology: html-css, javascript, workflow, security
  • 📊 Data: cia-data, riksdag-data, data-pipeline, schema
  • 🌍 I18n: i18n, translation, rtl
  • 🔒 ISMS: isms, iso-27001, nist-csf, cis-controls
  • 🏗️ Infrastructure: ci-cd, deployment, performance, monitoring
  • 🔄 Quality: testing, accessibility, documentation, refactor
  • 🤖 AI: agent, skill, agentic-workflow

For more information, see .github/labeler.yml.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

… tests

- Extract chrome/types.ts (interfaces: ChromeOptions, SiteChrome, BreadcrumbItem)
- Extract chrome/helpers.ts (depth, renderHreflangBlock, fallbackAlternateHref)
- Extract chrome/head.ts (renderChromeHead — SEO, OG, JSON-LD, hreflang)
- Extract chrome/header.ts (buildHeaderHtml — nav, CTA, breadcrumb, hero)
- Extract chrome/footer.ts (buildFooterHtml — columns, badges, scripts)
- Rewrite chrome.ts as thin façade delegating to sub-modules
- Fix `any` type in chrome JSON-LD findIndex (replaced with typed JsonLdNode)
- Add 59 comprehensive tests in chrome-decomposition.test.ts covering:
  - Sub-module isolation (no circular deps)
  - HTML5 structural correctness
  - WCAG 2.1 AA accessibility (skip-link, ARIA, roles)
  - RTL language handling (Arabic, Hebrew)
  - hreflang for all 14 languages
  - JSON-LD injection and XSS prevention
  - SEO meta tags (OG, Twitter Cards)
  - Language switcher (14 languages)
  - Hero banner control
  - Pagination rel links
- All 430 existing tests pass (zero regressions)

Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/e1a3c2db-9238-4874-8434-5932e8e4da99

Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
@github-actions github-actions Bot added testing Test coverage refactor Code refactoring size-xl Extra large change (> 1000 lines) labels May 6, 2026
…dence tests

- 8 tests for NewsArticle JSON-LD (required fields, author, publisher, provenance)
- 6 tests for BreadcrumbList JSON-LD (positions, validation, error handling)
- 4 tests for SpeakableSpecification WebPage JSON-LD
- 9 tests for markdown pipeline independence from chrome (GFM, sanitization)
- 3 tests for markdown security boundaries (XSS prevention)
- 3 tests for mermaid preprocessor
- Total: 33 new tests, all passing

Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/e1a3c2db-9238-4874-8434-5932e8e4da99

Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

Comment thread scripts/render-lib/chrome/header.ts Fixed
Copilot AI changed the title [WIP] Refactor architecture and improve test coverage for rendering pipeline feat: decompose chrome.ts into bounded-context sub-modules with comprehensive tests May 6, 2026
Copilot finished work on behalf of pethers May 6, 2026 11:16
Copilot AI requested a review from pethers May 6, 2026 11:16
@pethers pethers marked this pull request as ready for review May 6, 2026 11:18
Copilot AI review requested due to automatic review settings May 6, 2026 11:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Comment thread scripts/render-lib/chrome/header.ts Outdated
Comment on lines +15 to +17
import type { Language } from '../../types/language.js';
import { LANGUAGE_META, escapeHtml } from '../../generate-sitemap-html.js';
import { BASE_URL, GITHUB_BLOB, LANGUAGES } from '../constants.js';
Comment thread scripts/render-lib/chrome/footer.ts Outdated
* @license Apache-2.0
*/

import type { Language } from '../../types/language.js';
Comment thread scripts/render-lib/chrome/helpers.ts Outdated
*/

import type { Language } from '../../types/language.js';
import { LANGUAGE_META } from '../../generate-sitemap-html.js';
Comment thread scripts/render-lib/chrome/types.ts Outdated
readonly publishedIso?: string;
/** ISO-8601 date for `article:modified_time` / `og:updated_time`. */
readonly modifiedIso?: string;
/** JSON-LD blob(s) appended inside `<head>`. Already-stringified objects. */
Comment on lines +150 to +154
${hreflangHtml}

<link rel="sitemap" type="application/xml" href="/sitemap.xml">
<link rel="alternate" type="application/rss+xml" title="Riksdagsmonitor news (${escapeHtml(meta.nativeName)})" href="${opts.rssHref ?? '/rss.xml'}">
${pagerLinksHtml}
@pethers
Copy link
Copy Markdown
Member

pethers commented May 6, 2026

@copilot apply changes based on the comments in this thread

- Remove unused Language/BASE_URL imports from header.ts
- Remove unused Language import from footer.ts
- Switch all chrome sub-modules from generate-sitemap-html.js (CLI side effects)
  to scripts/sitemap-html/index.js (pure exports: LANGUAGE_META, escapeHtml)
- Fix jsonLd docstring in types.ts: clarify plain objects are expected (not
  pre-serialised strings) since head renderer calls JSON.stringify internally
- Align RSS href default in head.ts with footer: use per-language /rss_{lang}.xml
  for non-English (was always /rss.xml regardless of lang)

Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/971dbb8c-8f37-4274-962a-887ce3c118a1

Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 6, 2026

@copilot apply changes based on the comments in this thread

Applied all 5 review changes in 98f7a45:

  • Unused imports (Language, BASE_URL): removed from header.ts and footer.ts
  • CLI side-effects import: switched all 4 chrome sub-modules from generate-sitemap-html.js to scripts/sitemap-html/index.js (pure exports, no fs/console side effects)
  • jsonLd docstring: updated to clarify callers should pass plain JS objects — the head renderer calls JSON.stringify internally
  • RSS href alignment: head now uses the same per-language default as the footer (/rss_${lang}.xml for non-English, /rss.xml for English)

All 105 affected tests still pass, ESLint clean.

Copilot finished work on behalf of pethers May 6, 2026 11:47
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

@pethers
Copy link
Copy Markdown
Member

pethers commented May 6, 2026

@copilot failing checks
TypeScript & JavaScript Testing / Node.js Nightly Compat (pull_request)
TypeScript & JavaScript Testing / Node.js Nightly Compat (pull_request)Failing after 57s
TypeScript & JavaScript Testing / Unit Tests (Vitest) (pull_request)
TypeScript & JavaScript Testing / Unit Tests (Vitest) (pull_request)Failing after 1m

TypeScript type-check (scripts) was failing with TS6196 because SiteChrome
was imported but never referenced in the test file.

Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/df174842-32f7-4138-acaa-8f15122fb240

Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 6, 2026

@copilot failing checks
TypeScript & JavaScript Testing / Node.js Nightly Compat (pull_request)
TypeScri...

Fixed in aec5636. The TypeScript type-check (scripts) was failing with TS6196 because SiteChrome was imported in tests/chrome-decomposition.test.ts but never actually used. Removed the unused type import — both jobs now pass type-check.

Copilot finished work on behalf of pethers May 6, 2026 11:57
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🔍 Lighthouse Performance Audit

Category Score Status
Performance 85/100 🟡
Accessibility 95/100 🟢
Best Practices 90/100 🟢
SEO 95/100 🟢

📥 Download full Lighthouse report

Budget Compliance: Performance budgets enforced via budget.json

@pethers pethers merged commit 8ccac52 into main May 6, 2026
18 checks passed
@pethers pethers deleted the copilot/improve-html-rendering-pipeline branch May 6, 2026 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code refactoring size-xl Extra large change (> 1000 lines) size-xs Extra small change (< 10 lines) testing Test coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve architecture, code quality, and test coverage for HTML/XML/RSS/sitemap generation from article.md (render-articles pipeline)

3 participants