Conversation
…catch the next drift (#145) * fix(redesign): match hero headings by role, not by heading level RoyalRoad renumbered the redesign's chapter hero headings: the chapter title went h3 -> h1 and the fiction title h4 -> h2. Both selectors were pinned to the old levels, so on the beta layout: - chapterTitle matched nothing, and every recap failed with "Could not find the previous chapter's title"; - fictionTitle silently landed on the AUTHOR's h4 instead. That one produced no error at all — the recap was titled with the author's name, and findFictionOverviewUrl()'s closest("a") walk pointed the blurb fetch at /profile/<id> rather than the fiction. Match on role instead, which survives the next renumber: the chapter title is the hero's only heading that is not inside a link, and the fiction title is the heading inside the hero's link to the fiction. Scoping the latter to that anchor also fixes the overview URL by construction rather than by luck of document order. * ci(canary): cover every layout, run daily, file an issue on drift The drift above shipped unnoticed for a day because the canary could not see it and would not have said so if it could. Two gaps, both closed: Coverage. getChapterPageSelectors()/getFictionPageSelectors() default to FALLBACK_ADAPTER, so the canary only ever checked the legacy layout — the redesign's selectors were never tested against the live site at all. It now loops over ADAPTERS and asks RoyalRoad for each layout in turn via the beta-ui-v2 cookie, asserting first that the requested layout is what was actually served so a withdrawn beta cannot masquerade as drift. Cadence. It ran on pull_request only, and RoyalRoad changes its markup on its own schedule rather than ours. Added a daily cron; a failing scheduled run opens (or comments on) one labelled issue, which is closed automatically when the canary goes green again — a red X on a cron run notifies nobody. Two further hardening changes fall out of the same bug. Presence checks cannot see a selector that matches a real but WRONG element, so the two identity selectors are now checked for what they resolve to: the chapter title against the page's <title>, and the fiction title's closest("a") against /fiction/. Under the old selectors that second test fails while "fictionTitle selector exists" still passes. And selectors must now match exactly one element unless listed in MAY_REPEAT, since matching several is how the extension's querySelector starts reading the wrong one. The canary no longer loads the extension, so the workflow's build step is gone with it — one less way for this job to fail for reasons that have nothing to do with selectors. * refactor(selectors): anchor on semantic hooks instead of position and colour Audit of every remaining selector on both layouts against the live site, after the hero-heading drift showed what the fragile ones look like. Each replacement below was verified across six different fictions. Legacy: chapterTitle h1.font-white -> .fic-header h1 fictionTitle h2.font-white -> .fic-header a[href*='/fiction/'] :is(h1..h6) closeButton a four-deep nth-child chain -> #settings .modal-footer button[data-dismiss='modal'] reportPlacement div.col-lg-3:nth-child(3) -> div:has(> a[href^='/report/chapter/']) Redesign: prevChapterBtn now matches EITHER RoyalRoad's data-vt-direction='prev' or the Font Awesome arrow, so losing one hook is survivable togglePlacement [class*='grid-cols-2']:has(#chapterSelect) — the same element prepareMounts() resolves, so the override hint and the behaviour finally agree, and it stops matching the duplicate nav bar below the chapter closeButton RoyalRoad's data-rr-dialog-close, not "the direct child that happens to be a button" font-white and the nth-child chains work today but say nothing about what they select; the anchor-scoped fiction title additionally makes findFictionOverviewUrl()'s closest("a") walk correct by construction rather than by luck of nesting — the legacy layout had the same latent author-heading trap the redesign fell into, one markup change away. Deliberately unchanged: blurbLabels still leans on .font-red-sunglo, which appears ~60 times on an overview page, because RoyalRoad gives the content-warning box no id, data attribute or semantic class to aim at. Fixtures now carry a .fic-header block with the author heading present, mirroring the real page, so a title selector that drifts onto the author fails in the suite. togglePlacement leaves the canary's MAY_REPEAT allowlist: it matches exactly one element on both layouts now. * ci(canary): gate the drift-issue automation on the canary's own result Two ways the scheduled run's issue automation reported the wrong thing. The close step ran on success(), but the canary calls test.skip() on every test when RoyalRoad is unreachable, and Playwright exits 0 on an all-skipped run (verified: stats.expected 0, exit 0). So an outage during the 05:17 cron auto-closed a still-valid drift issue with "the canary is green again", having checked nothing. It now runs the JSON reporter alongside list and closes only when stats.expected is above zero. The report step ran on failure() of ANY prior step, so a lockfile drift or a Playwright download hiccup filed "RoyalRoad's layout drifted" and sent someone into the adapters after a CI problem. Both steps now key off steps.canary.outcome instead of the job's. * test(canary): pin the browser it asks RoyalRoad with, and make the layout guard bite browser.newContext() does not inherit the project's `use` options — Playwright applies those in the context/page fixtures, which a shared per-describe navigation can't use. The canary was therefore fetching royalroad.com as HeadlessChrome rather than the Desktop Chrome the config asks for, so anything the site varies on the user agent would have surfaced as a selector drift on a required check. The "RoyalRoad served this layout" test claimed to guard the tests below it, but Playwright tests are independent and nothing skipped on its failure: a renamed cookie would have failed it AND every selector test with a "drifted" message, which is the exact masquerade the split exists to prevent. The check now also runs in beforeAll and sets a flag the selector tests skip on, while the test itself stays reportable. Also close the context on the paths that never hand a page back — the caller closes contexts through the page it was given, so an outage leaked one context per describe per retry until the worker exited. * fix(redesign): let togglePlacement actually move the toggle prepareMounts() never read selectors.togglePlacement: it resolved the nav bar from a hardcoded #chapterSelect walk with a #chapterHeroData fallback, so resolveToggle() never returned null and the base mount was dead code. A user whose toggle placement broke could edit the key in Advanced Settings — which the settings UI offers precisely so a layout change can be worked around without an extension update — and watch nothing happen. The configured selector is now asked first, with the #chapterSelect walk as the fallback for when it stops matching. Both resolve to the same inner grid on today's markup, so the rendered placement is unchanged.
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.
Summary
Patch release. RoyalRoad renumbered the redesign's chapter hero headings, which broke every recap on the beta layout; the canary that should have caught it was only checking the legacy layout. Both are fixed.
User-visible changes
chapterTitlewas pinned toh3and matched nothing after RoyalRoad's renumber, so every recap failed with "Could not find the previous chapter's title".fictionTitlesilently landed on the author's heading, sofindFictionOverviewUrl()'sclosest("a")walk fetched/profile/<id>— no error, just the wrong text.togglePlacementis honoured on the redesign.prepareMounts()never read the key, so an Advanced Settings override did nothing. Rendered placement is unchanged.Both hero selectors now match by role rather than heading level, which survives the next renumber. The remaining selectors on both layouts were re-anchored on semantic hooks instead of position and colour in the same pass.
Maintenance
ADAPTERSvia thebeta-ui-v2cookie, asserts the requested layout was served, runs daily, and files one labelled issue on drift.MAY_REPEAT.stats.expected > 0.HeadlessChrome, and no longer leaks a context per describe per retry during an outage.Testing
blurbLabels, intentionally unconfigured).svelte-checkand Prettier all green.pnpm zip:firefox && pnpm verify:sources—✔ Every file matches, 14 files compared.Merge with a merge commit, not a squash. Squashing
dev→mainputs a commit onmainwhose tree matchesdevbut whose history doesn't, destroying the merge base — the next release PR then lists every commit since the last real ancestor andmainneeds a hand-repaired reconcile back-merge. The repo already carries two of those scars (b6ba92f,313fbb9).