Achhina's Digital Garden

2026-07-25 OpenRouter Interceptor Architecture Refactor

12 min read

2026-07-25 OpenRouter Interceptor Architecture Refactor

What I set out to do

Open-ended: asked for an assessment of design, architecture and code quality on an extension I use daily, then picked four items off the resulting list to actually do.

What I actually did

Five commits on refactor/extract-character-card, not merged.

The shape of the problem. Two generations of code, easy to tell apart. The newer layer (capture-store.ts, message-router.ts, the buildXExpr() modules) is pure, documented with the why, dependency-injected and tested. The older layer is two files: sidebar.ts (2516 lines) and background.ts (2251), 21% of src/ and nearly all the untested logic. Every recommendation reduced to “keep applying the pattern you already proved” — message-router.ts was the existence proof that this code can be refactored well.

Character card extraction (3fdb16e). background.ts 2251 → 1552. Format detection, four notation parsers (W++, PList, Ali:Chat, XML), section extraction, card assembly and filenames into src/character-card/, which touches no browser API. Three near-identical section extractors collapsed to one table with per-section stop words. Before deleting the originals, diff-tested the new extractors against them over 510 generated prompts; all four functions agreed on every one.

Deduplication (same commit). generateUUID, hashString, generateCharacterCardFilename and the ~40-field chara_card_v2 literal each existed twice. Two judgment calls: kept the space-based filename convention over the underscore one because it produced the existing card library and switching would double every card on disk; and left showToast duplicated, because the sidebar stacks toasts and the options page replaces a single one, with different CSS in different stylesheets. Forcing a shared abstraction over deliberately different behaviour would have been worse than the duplication.

Sidebar split (3678d85). 2516 → 180 lines. The DOMContentLoaded closure became four feature modules (captures, queries, diagnostics, playground), each binding its own DOM and owning its own state, with genuinely cross-cutting state in state.ts. captures and queries each need one thing from the other; those two references are tied together in the composition root rather than by importing across features. ui/markdown.ts, playground/parse-markdown.ts and theme.ts fell out as pure and now-tested.

The settings that did nothing (c160a82). Five settings the options page persisted and the background never read. Wired four: retentionDays (the sweep hardcoded 7 days and ignored the dropdown), captureFullPayload, domainFilter/domainList. Removed maskUserData instead of implementing it — its description promises to mask “names and addresses”, which a regex cannot deliver, and shipping an approximation under that label is a privacy claim the extension cannot keep.

The parser bug (a11d10e). Found during extraction, deliberately deferred, then fixed on request. A Header: section has no closing delimiter, so its capture ran until a newline followed by another keyword, or end-of-input — but the end-of-input branch sat inside the newline group, so a section written as the last line without a trailing newline never closed and was silently dropped. Moving it out fixes that but on its own makes mixed-format prompts worse, since the stop-word lookahead only recognised a bare keyword; a capture that used to fail would now swallow a following ## Personality block. So the lookahead also learned the decorated forms authors write. Measured over 543 prompts × 3 extractors: 1205 identical, 419 newly found, 5 corrected, 0 regressions.

The tests that could not fail (4723ab2). Ten Playwright tests worked by readFileSync-ing a .ts file and grepping it for substrings. Four broke during the sidebar split without a single behaviour changing, and one had been failing since 016c089 renamed a constant it asserted on. Replaced with a sidebar lab following the carousel lab already in the repo: an esbuild bundle of the real feature modules with only browser stubbed, loaded into a page generated from src/sidebar/sidebar.html. 22 behavioural specs. Also added openrouter-service.test.ts (12) and five DOM tests for the analyse buttons.

What was striking

  • The test suite punished the refactor and caught none of it. Four greps went red while behaviour was provably unchanged; meanwhile I mutated the real source four ways (never disabling the reload button, dropping the api-key check, not persisting the tab, counting captures instead of distinct sites) and every mutation would have sailed past them. A test that reads source text has negative value: it costs refactoring effort and buys no safety. The new specs caught all four, each by exactly one test.
  • Generating the lab page from the shipped HTML is the whole trick. build-sidebar-lab.mjs reads src/sidebar/sidebar.html and swaps the script tag. Rename an id and the tests fail, rather than passing against a fixture that quietly drifted. The pre-existing “good” markdown test reimplements renderMarkdown inline in the test HTML, so it tests a copy — same failure mode, better disguised.
  • Two fixes were needed where I expected one. The trailing-newline fix in isolation would have regressed mixed-format prompts. Measuring the corpus before and after is what surfaced that; the instinct to ship the one-character regex change would have traded 419 fixes for an unknown number of new breakages.
  • The 5 “corrections” were the interesting number. Prompts carrying both traits: and Personality: used to return the alias value, because the primary pattern failed to close and fell through. Nobody would have reported that as a bug; it just quietly picked the wrong field.
  • Removing a feature was the right answer once. Four of the five dead settings were wiring jobs. maskUserData was not, and the honest move was deleting the checkbox rather than shipping a regex that implies protection it cannot provide.
  • test/README.md was entirely fiction. Described Jest, Puppeteer, test/e2e/, fixtures.js, setup.js and four npm scripts, none of which exist. Rewrote it. Stale docs that confidently describe a different project are worse than no docs.
  • Adam’s debug logging was not the mess it looked like. I flagged 171 console.log calls as noise; they feed the diagnostic-capture button, which is the whole debugging workflow. Left every one alone. Worth asking what a smell is for before proposing its removal.

Verification

646 unit tests (from 480), 47 files. Playwright 259 passed / 0 failed (from 231 passed / 2 failed on main). tsc --noEmit clean across all three projects, ESLint and Stylelint clean, npm run build clean, after every commit. Behaviour preservation for the extraction established by differential testing, not inspection. New browser tests validated by deliberate mutation of the real source.

Second session: the design pass

Merged the refactor branch (e046eea, --no-ff), then Adam asked the same open question about design rather than architecture. Rendered the real sidebar through the lab at 320px and 400px in both themes and measured, rather than reading CSS and guessing. That turned an aesthetic question into a defect list.

What measuring found. Three files disagreed about where data-theme lives: theme.ts wrote it to <body>, diagnostics.ts read it from <html>, captures-list.ts selected on :host([data-theme]). Only the writer was right, so both readers silently got null and fell back to a default. The visible cost: the dark-theme rules for the {{user}} and {{char}} spans never matched, leaving light-theme blue and green on a near-black code block at 1.80:1 and 2.23:1. Separately --primary-color was never overridden for dark and is used as text, putting stat values at 2.83:1 and the active tab at 3.45:1.

The fix (ec55233). One home for the attribute (root element, exported as THEME_ATTRIBUTE_TARGET so the choice is stated once); highlight colours become custom properties, which are the only part of the cascade that crosses a shadow boundary; --primary-text split from --primary-color because the accent-as-text and the accent-as-fill-behind-white cannot lighten together. options.ts dropped its copy of the theme manager and imports the shared one, which moved to utils/. After: 5.16, 5.89, 5.75, 7.02. Light theme renders pixel-identically.

What was striking (second session)

  • Rendering it was the whole difference. The first assessment read source and produced architecture findings. Ten minutes of screenshots produced a defect the architecture pass could never have surfaced, because every colour literal in the stylesheet was already correct. The bug was in a selector that never matched.
  • :host([data-theme]) is a trap. It parses, ships, and silently never matches, because a shadow root cannot select on an ancestor’s attributes. There is no error and no warning. Custom properties are the only mechanism that works, and the component was already using them correctly for --text-primary three lines above.
  • The bug had a second victim I nearly missed. diagnostics.ts reading the wrong element meant every diagnostic bundle Adam has ever captured reported system-dark/system-light and could never report an explicit choice. The debugging tool was quietly lying, and nothing about the visual symptom pointed at it.
  • My own test was flaky for a reason worth remembering. The first run after the fix still failed at 3.64:1. Not a bad fix: .tab-button carries transition: all 0.2s, so applying the theme animates the colour and the test sampled mid-transition. Spent a while chasing a phantom stale-stylesheet theory before the number made sense as an interpolation between 3.45 and 7.02. Tests that read computed colour must disable transitions first.
  • Assert on measured contrast, not on colour literals. Written into AGENT.md along with the theming rules, since a literal-based test would have passed throughout this entire bug.

Third session: the layout group

Adam picked “compact in place” for the chrome and “badge only when notable” for the status pill, from a set of options offered with ASCII previews. Previews were worth the effort here: the choice was spatial, and a paragraph describing 275px versus 166px does not land the way two boxes side by side do.

Chrome 275px → 166px (39% → 24% of a 700px sidebar). Stats collapsed from three stacked columns to one line, the JanitorAI section heading went (the buttons name themselves), and both clear buttons moved into a header overflow menu. Destructive actions now recede: a red label that only fills on hover, rather than a saturated full-width bar sitting above every capture.

Three defects fixed alongside: complete captures no longer render a status badge (it said nothing and held the width the title needed, so titles truncated first); expanding a row now hides the truncated preview that had been duplicating the full prompt right below it; the queries model select got its own row, which fixes both the clipped model name and the wrapped view-capture arrow.

What was striking (third session)

  • The duplicated inline <style> blocked the work, which is how it got fixed. Item 9 was not in the selected group, but the inline block in sidebar.html shadowed the exact .janitorai-section padding I needed to change. A latent duplication that costs nothing until it blocks you is the normal way this debt gets paid: not when you notice it, but when it stands in the road.
  • Every rule in that block was already in sidebar.css, and the inline copy silently won. The stylesheet’s JanitorAI button padding had been dead for as long as both files existed. Kept the inline values when deleting, so nothing moved visually.
  • My own new contrast rule caught my own new element. The menu’s Clear All measured 3.70:1 in dark, because --error-color has no dark value for exactly the reason --primary-color did not. Added --danger-text and put the menu item in ACCENT_TEXT. A rule written one session earlier stopped me shipping the same class of bug one session later, which is the entire argument for encoding rules as tests rather than as intentions.
  • The stale fixture bit twice. sidebar-test.html is a 724-line hand-copy of the sidebar that two legacy specs (813 lines) drive. Removing .actions-section from it broke 40 playground tests, because the fixture’s copy of that section also held a #playgroundToggle that the shipped sidebar has not had since the playground became a tab. The fixture is not just stale, it is preserving UI that was deleted long ago and pretending to test it.
  • Two legacy assertions were describing nothing. One checked the clear buttons do not overlap horizontally; one asserted every capture carries a status badge. Both now describe a design that does not ship, and both are superseded by lab tests that drive the real modules.

Design findings not yet acted on

Reported and awaiting Adam’s call, since these are UX judgment rather than defects:

  • The palette is copy-pasted four times per file across two files, which is why adding one token to fix dark mode meant editing eight blocks. Adding --danger-text in the third session cost four more edits, so this compounds every time.
  • An empty grey square stands in for a missing avatar and reads as a permanently-loading image. An initial-letter tile would say more.
  • Chat with Mira - JanitorAI sits directly above janitorai.com; the suffix is redundant with the line under it.

Done in the third session: chrome compaction, destructive-action demotion, the duplicated prompt on expand, the queries clipping, the COMPLETE badge, and the inline <style> duplication.

Open follow-ups

  • Worth a run in the real PWA profile, since Playwright never loads the Firefox extension for real (the fixture notes it must be installed manually via about:debugging).
  • Still no CI; the local repo has no GitHub remote, so the pre-commit hook remains the only gate and npm test runs only when remembered.
  • background.ts is still 1569 lines. The JanitorAI response filtering and the two inline executeScript blobs (unescaped URL interpolation in one) are the next candidates.
  • src/types/global.d.ts is still 335 lines of ambient globals with no import graph.
  • design/sidebar-layout is unmerged (bb00b69). fix/theme-contrast merged as 9e9a74f.
  • test/playwright/sidebar-test.html should be retired. 724 lines of hand-copied sidebar markup driving 813 lines of spec, already preserving deleted UI (#playgroundToggle). The sidebar lab exists and does this properly; converting the two specs is the obvious next test job.
  • The palette duplication is the remaining design item that compounds with every new token.

Graph View

Backlinks