Skip to content

Fix generated ruleset and FT stylesheet rewrite#12

Open
andesco wants to merge 1 commit into
everywall:mainfrom
andesco:codex/fix-generated-ruleset
Open

Fix generated ruleset and FT stylesheet rewrite#12
andesco wants to merge 1 commit into
everywall:mainfrom
andesco:codex/fix-generated-ruleset

Conversation

@andesco
Copy link
Copy Markdown
Contributor

@andesco andesco commented May 27, 2026

Summary

  • fix stale generated ruleset entries for NYT/Time user-agent and Tagesspiegel tests
  • make the FT stylesheet rewrite guard against empty and already-absolute href values
  • keep the generated ruleset in sync with the source rule fixes

Validation

  • parsed ruleset.yaml and affected source YAML files with yaml parser

Copilot AI review requested due to automatic review settings May 27, 2026 01:40
Copy link
Copy Markdown

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

Note

Copilot was unable to run its full agentic suite in this review.

This PR fixes and hardens ruleset behavior by correcting YAML formatting/typos and making the FT stylesheet “de-proxy” logic safer.

Changes:

  • Guard against missing href and only de-proxy proxied stylesheet URLs to avoid breaking absolute URLs.
  • Fix a typo in the user-agent header key.
  • Fix/clean YAML test list indentation and remove trailing whitespace.

Reviewed changes

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

File Description
rulesets/gb/ft-com.yaml Makes the FT stylesheet URL rewrite conditional and null-safe; fixes test indentation.
ruleset.yaml Fixes user-agent header typo, corrects YAML test list formatting, and aligns FT rewrite logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ruleset.yaml
Comment on lines +235 to +239
// Only de-proxy stylesheet URLs ("/https://...") to avoid breaking absolute "https://..." hrefs.
if (href.startsWith('/https://') || href.startsWith('/http://')) {
const updatedHref = href.slice(1).replace(/(https?:\/\/.+?)\/{2,}/, '$1/');
el.setAttribute('href', updatedHref);
}
Comment thread ruleset.yaml
if (!href) return;
// Only de-proxy stylesheet URLs ("/https://...") to avoid breaking absolute "https://..." hrefs.
if (href.startsWith('/https://') || href.startsWith('/http://')) {
const updatedHref = href.slice(1).replace(/(https?:\/\/.+?)\/{2,}/, '$1/');
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants