Share the Wikimedia Commons plumbing via @spelling-creator/core - #17
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
This was referenced Aug 5, 2026
The web app and the MCP server each carried their own Commons client. They were never actually interchangeable — different exports (fetchWikimediaImage vs resolveWikimediaImage), different hit shapes, paging only in the browser, and different error wording — so this does not collapse them into one function. Instead the parts that genuinely are the same move to core: the endpoint, the query/unwrap round-trip, search-rank restoration, the image/non-image filter, and the licence/author handling Commons' attribution norms require. Each app keeps a thin adapter, so its public contract and its user-facing strings stay its own. Note this is not a net line saving (+73). The win is that the attribution and API-shape rules now have one definition instead of two copies that had already drifted apart. Two runtime differences are handled by injection rather than duplication: the MCP server passes the User-Agent that Wikimedia's policy requires from datacenter egress, and the web app passes a DOMParser-based HTML stripper. Core itself stays DOM-free, which the worker-env lint rule now enforces, since the Worker pulls this in through @spelling-creator/mcp/worker. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
playforge-coding
force-pushed
the
core-dedupe-wikimedia
branch
from
August 5, 2026 23:30
18aefbc to
0c1a336
Compare
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
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.
Second layer. Stacked on #16.
The premise I started with was wrong
I'd scoped this as "web 215 lines vs mcp 177, identical export surface, collapse to one module." That came from a crude grep that only matched
export async function. The two clients are not interchangeable:fetchWikimediaImage(hit)resolveWikimediaImage(ref){hits, total, totalHits}{id, title, tags, descriptionURL, licenseURL, …}{ref, description, source, …}gsroffset)Search failed (404).Wikimedia Commons request failed (404).Those differences are part of each app's contract — the image dialog's shape and the MCP tool's documented shape — so collapsing them would have been a breaking change dressed up as a refactor.
What this does instead
Moves only what is genuinely common to
@spelling-creator/core/wikimedia: the endpoint, the query/unwrap round-trip, search-rank restoration, the image/non-image filter, and the licence/author handling Commons' attribution norms require. Each app keeps a thin adapter, so its public shape and its user-facing strings stay its own.Two runtime differences are handled by injection rather than duplication:
nodegets throttled or 403'd);Core staying DOM-free matters here: the Worker pulls this module in through
@spelling-creator/mcp/worker. Theworker-env lint rule added in #16 now enforces that.This is not a line saving
Net +73 lines. The win is that the attribution rules and the Commons API shape have one definition instead of two copies that had already drifted. If you'd rather carry the duplication than the indirection, this layer can be dropped without affecting #16.
Verification
pnpm run lint,pnpm run build,pnpm run build:docs, and all three suites pass. The mcpadd_imagetest exercisesresolveWikimediaImageend to end, so the refactor has real coverage.Stack created with GitHub Stacks CLI • Give Feedback 💬