chore(registry): remove low-use catalog items and variant tails - #4274
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
jrusso1020
left a comment
There was a problem hiding this comment.
Requesting changes at 4748a2fb for one un-updated reference. The deletion itself is clean — this is a single line that the second commit ("remove stale references to deleted catalog items") was aimed at and missed.
Blocking: .github/workflows/catalog-previews.yml:96 names a block this PR deletes.
if [ -n "$RENDERER_CHANGED" ]; then
CHANGED_ITEMS=$(printf '%s\n' $CHANGED_ITEMS \
code-snippet-visual-studio-dark code-snippet-apple-terminal-pro | sort -u)
ficode-snippet-visual-studio-dark is gone at this head — the directory is deleted and registry/registry.json mentions it zero times. The path from there to a red X is unconditional:
- Any future PR touching
scripts/generate-catalog-previews.ts,scripts/registry-target-paths.mjsorscripts/catalog-preview-temp.tssetsRENDERER_CHANGED, which force-adds both canary names. - Each name runs
npx tsx scripts/generate-catalog-previews.ts --only "$item" --skip-video. discoverItemsends withif (nameFilter && items.length === 0) failItemNotFound(nameFilter);, andfailItemNotFoundisprocess.exit(1).- The workflow's loop counts that (
if ! timeout 120 … then FAILED=$((FAILED + 1))) and finishesif [ "$FAILED" -gt 0 ]; then … exit 1.
What makes this worth blocking rather than noting: this PR's own CI stays green, because it doesn't touch the three renderer scripts. The failure lands on whoever next edits them, in a workflow they have no reason to connect to a catalog deletion two weeks earlier.
On picking the replacement — the comment above the line says the pair is deliberately two different mount shapes, "a <template> (mounted through a wrapper) and one that registers its timeline at body level," and that getting it wrong is silent. Counting <template> in each block's html as a rough proxy:
| block | <template> |
state |
|---|---|---|
code-snippet-visual-studio-dark |
0 | deleted here |
code-snippet-apple-terminal-pro |
0 | survives |
code-snippet-dark-2026 |
1 | survives |
code-snippet-dark-modern |
1 | survives |
So code-snippet-dark-2026 (or -dark-modern) is the substitution that gives the pair genuine coverage of both shapes. Worth flagging that by this proxy the original pair was 0 and 0 — if that holds up against however you actually distinguish the two shapes, the canary may have been covering one shape twice all along, and this is a chance to fix that rather than restore it.
Checked and clean — noting these so nobody re-runs them:
- No other dangling references. I grepped the whole tree at head for all 29 removed item ids. Three classes of hit came back, all false positives:
motion-blurhits are the componentregistry/components/motion-blur/, which survives (onlyregistry/examples/motion-blur/was removed, andmotionBlurShutter.browser.test.tscorrectly drops just that one path);slideshow-demois astatic.heygen.aivideo URL indocs/guides/slideshow.mdx, unaffected by removing the registry example; and the 24 ids inpackages/cli/src/registry/__fixtures__/wordpiece-reference.jsonare tokenizer input text in a[{text, ids}]golden corpus with no registry coupling —wordpiece.test.tsonly re-encodes each string and compares ids. airbnb-deck,startup-pitchandvscode-theme-visualizerhave zero references left. Movingairbnb-deck/demo.htmltopackages/core/src/runtime/fixtures/deck-audio-demo.htmlto keepdeckAudio.test.tsalive is the right call.
Non-blocking: the check-docs-catalog.mjs success message now under-reports.
Narrowing the assertion to pages still live in registry.json is the right change — a deliberately retired item should take its page with it. But the line below still prints PASS no page lost: ${before.length} pages before, ${after.length} after on a run that just lost 24 pages on purpose. The gate's own output is the thing a future reader will trust. Since you already compute missing, printing the deliberate drops (missing.length - unexpectedMissing.length) alongside the "legitimately new" count would keep the message honest.
— Rames
|
Replaced the deleted canary with surviving code-snippet-dark-modern and pushed b68ce15; please re-review. |
b6f6c1d to
64aec56
Compare
|
Rebased the removal branch onto main at 24c3e19, regenerated the local search vectors and registry artifact revision, and reran the catalog, Fallow, and deletion guard checks on the rebased head. |
jrusso1020
left a comment
There was a problem hiding this comment.
Re-review at 4582f68d765c33f0e0b45e8e04b758b14fc845db, anchored on merge-base(24c3e197e, HEAD) = 24c3e197e (.base.sha is an ancestor of head, and the local file count reconciles against the API exactly: 195 files, +863/-48141).
My previous block is resolved. .github/workflows/catalog-previews.yml:96 now force-adds code-snippet-dark-modern instead of the deleted code-snippet-visual-studio-dark. Verified the replacement survives rather than taking the commit message's word for it: registry/blocks/code-snippet-dark-modern/ is present at head, it is in registry.json, and it is not touched anywhere in this PR's diff. code-snippet-apple-terminal-pro likewise survives, and registry/blocks/code-snippet-visual-studio-dark/registry-item.json is confirmed D. Worth noting you avoided the trap I flagged last time: render-entries/dark-modern.html is deleted here, but that file belongs to the vscode-theme-visualizer example, not to the surviving code-snippet-dark-modern block.
Blocking — the rebase left registry/registry.json stale, and it silently reverts #4277
The rebase moved the merge base from d5e00c0e to 24c3e197e, which pulled in 01734fae3 feat(registry): colorama-wipe (#4277). Every other generated artifact came along correctly. registry/registry.json did not: at head it is the pre-#4277 file with this PR's 25 removals applied.
Item sets from registry.json at each anchor:
| ref | items | colorama-wipe |
week-in-merges |
|---|---|---|---|
old merge-base d5e00c0e |
417 | absent | present |
old head 4748a2fb |
392 | absent | present |
new merge-base 24c3e197e |
417 | present | absent |
new head 4582f68d |
392 | absent | present |
origin/main 53153d4a |
417 | present | absent |
#4277's net effect on the index was exactly one in, one out (417 → 417: +colorama-wipe, -week-in-merges). Head undoes both. So relative to its own merge base this PR removes 26 items, not the 25 it intends — the 26th is colorama-wipe, merged to main earlier today — and it re-registers week-in-merges, which #4277 had de-indexed.
Two things make this hard to see:
- Neither name appears in the PR's file-level diff.
registry/components/colorama-wipe/andregistry/blocks/week-in-merges/are untouched on disk. The regression exists only in the index, so it shows up by diffing item sets, never by reading the file list. registry.jsonis the only stale artifact.docs/public/catalog-index.json,docs/docs.jsonandregistry/catalog-artifact/local-vectors.{json,bin}all carry #4277's state correctly at head — I checked each one at both the merge base and the head. So after merge,registry.jsonandcatalog-index.jsondisagree about which items exist: the catalog index hascolorama-wipeand notweek-in-merges; the registry index has the reverse.
Consequence as it stands: colorama-wipe ships with its files present and no registry entry, and week-in-merges is re-indexed after main deliberately dropped it.
Nothing in CI covers this, so the green is real and so is the drift. check:catalog-drift compares generator output against the committed tree for docs/public/catalog only — scripts/catalog-drift.ts:46-57 does git archive HEAD docs/public/catalog, and the comparison never reads registry/registry.json. verify:catalog-payloads is --changed origin/main-scoped. I pulled the check runs at this head and there are zero non-success conclusions; the detector for this class of drift is pointed at a different tree.
Fix is to regenerate registry.json on top of 24c3e197e and then assert the item-set delta against the merge base is exactly your intended 25 removals and nothing else. That last assertion is the part worth keeping — it is the check that would have caught this.
Non-blocking — one of the two new .fallowrc.jsonc entries is in the wrong list
packages/cli/src/registry/motionBlurShutter.browser.test.ts went into ignorePatterns. Per ci.yml:231, fallow audit runs dead-code + complexity + duplication, and ignorePatterns exempts a file from the whole audit. The comment you wrote next to it states a duplication-only reason ("intentionally contains repeated inline scenarios for distinct failure paths"), so the exemption is three analyses wide where the rationale is one wide, and it stays that way for every future change to that test.
The narrower mechanism already exists in the same file: duplicates.ignore (.fallowrc.jsonc:468) holds ~40 entries, several of them for precisely this situation — sourcePatcher.ts, useGsapSelectionHandlers.ts and hfIds.ts are all there with comments saying a line shift made fallow re-flag a pre-existing clone, which is what a one-line deletion in a parametrized array would do here. Moving the entry across is a one-line change.
The companion entry in the same commit is correctly placed: packages/core/src/runtime/fixtures/deck-audio-demo.html is an R095 rename of registry/examples/airbnb-deck/demo.html, so it left the registry/** umbrella and now needs its own ignorePatterns line. That one is a genuine whole-file exemption for a non-code fixture.
Smaller note on the same commit: fix(ci): scope fallow audit for catalog fixtures describes neither file — one is a core runtime fixture renamed out of the catalog, the other is a CLI browser test.
I could not execute fallow here (no bun, and no node_modules in this checkout), so this finding is read off the config and ci.yml, not off a run. I am not disputing that the entry is needed — only where it belongs.
Checked and fine
ALLOWED_DELETIONSis an exact set match. The 172 paths added toscripts/check-no-main-deletions.mjsare precisely the 172 paths this PR deletes — zero deleted-but-unlisted, zero listed-but-not-deleted. (My first pass reported one gap onvscode-themes/LICENSE; that was my own extraction regex dropping an extensionless filename, not a real gap.)- The rebase did not silently alter the other 192 files. Of the paths in the original scope, only five blobs differ between the old and new heads, and all five also moved upstream between the two merge bases — so they are rebase pull-ins, not new edits.
registry.jsonis the one where the pull-in did not actually land, which is the blocker above. motionBlurShutter.browser.test.tsdrops exactly one line,registry/examples/motion-blur/index.html, from a parametrized list. Correct and minimal: its other two paths point atregistry/components/motion-blur/, which survives.- Scope grew by exactly three files versus the head I blocked, and nothing left scope.
Heads-up, not a finding
#4276 is still open at 7a098147 and touches registry/registry.json and both catalog-artifact/local-vectors files. Whichever of #4274 / #4276 lands second will need its artifacts regenerated on the new base — and given what happened here, that regeneration is the step to verify rather than assume.
— Rames
|
Fixed the rebase artifact drift by regenerating registry.json against merge base 24c3e19, restoring colorama-wipe and removing week-in-merges; added a merge-base set-delta guard wired into check:catalog-drift, and moved the motionBlurShutter duplication exception into duplicates.ignore. |
jrusso1020
left a comment
There was a problem hiding this comment.
Re-review at c83d8bda9ddcbd5e03d5a7c59a36dc19631fbca9. Anchored on merge-base(24c3e197e, HEAD) = 24c3e197e — .base.sha is an ancestor (rc=0), so this is a fresh commit on the same base rather than another rebase. Local numstat reconciles against the API exactly: 197 files, +887/−48138. Scope grew by exactly two paths versus the head I blocked (package.json, scripts/check-registry-set-delta.mjs); nothing left scope.
My block is resolved
I re-derived this from the manifests rather than taking the commit message for it:
| anchor | items | colorama-wipe |
week-in-merges |
|---|---|---|---|
merge-base 24c3e197e |
417 | present | absent |
old head 4582f68d |
392 | absent | present |
new head c83d8bda |
392 | present | absent |
Item-set delta against the merge base is 25 removed, 0 added, and the 25 are exactly the intended retirements (21 code-snippet-*, 2 liquid-glass-*, vscode-theme-visualizer). #4277's one-in-one-out is preserved. The registry/registry.json edit is 4 lines in, 4 lines out — the minimal correction, in sorted position.
The .fallowrc.jsonc nit is also addressed: motionBlurShutter.browser.test.ts moved out of ignorePatterns into duplicates.ignore.
Non-blocking — the new guard enforces a different model of the manifest than the generator that writes it
scripts/check-registry-set-delta.mjs works, and I proved it is load-bearing before criticising it. Reverting registry/registry.json to the 4582f68d version and running the guard reproduces my blocking finding verbatim:
Registry item-set delta mismatch against origin/main:
missing from registry.json: colorama-wipe
unexpected in registry.json: week-in-merges
rc=1
That is the right failure, and it is the check I asked for. The problem is the model it encodes. The guard computes expected = baseNames − removed, where removed comes only from --diff-filter=D on registry/{blocks,components,examples}/*/registry-item.json. There is no added set, so any name in the head manifest that is not in the base manifest lands in unexpected and exits 1.
scripts/generate-registry-items.ts:161-181 is the thing that actually writes this file, and its rule is different — and its own docblock says so: "The manifest lists every item on disk". It readdirs each type dir and emits every subdirectory containing a registry-item.json. No allowlist, no base, no diff.
Three results, all from running code rather than reading it:
-
A legitimate addition fails. Adding
registry/blocks/zz-new-test-block/registry-item.jsonplus its manifest entry →unexpected in registry.json: zz-new-test-block,rc=1. -
It would have failed #4277 itself — the PR whose loss my block was about, merged this morning. Running this guard at
01734fae3against its parent:Registry item-set delta mismatch against 9603fe16e: missing from registry.json: week-in-merges unexpected in registry.json: colorama-wipe rc=1Both halves fire.
colorama-wipebecause it was added (A registry/components/colorama-wipe/registry-item.json), andweek-in-mergesbecause #4277 dropped it from the manifest without deleting its directory — so it never appears in--diff-filter=Dand the guard still expects it. -
The two models already disagree on
maintoday, and not because of this PR:ref generator would emit manifest has drift parent of #4277 9603fe16e417 417 — merge-base 24c3e197e418 417 week-in-mergesorigin/main5ca2e7838418 417 week-in-mergesregistry/blocks/week-in-merges/{registry-item.json,week-in-merges.html}are still on disk on main. This drift is inherited from #4277, not introduced here — your manifest is exactly merge-base minus 25 and is internally consistent. But it means the guard you are adding will reportunexpected in registry.json: week-in-mergesagainst anyone who repairs that drift by regenerating.
The fix for the addition class is small — derive the additions from the same diff and widen expected:
const addedPaths = run(["diff", "--name-only", "--diff-filter=A", base + "...HEAD"])
.split("\n")
.filter((path) => ITEM_RE.test(path));
const added = new Set(addedPaths.map((path) => path.split("/")[2]));
const expected = new Set([...baseNames, ...added].filter((name) => !removed.has(name)));The de-index-without-delete class is a policy question rather than a bug: either the manifest is tree-derived (in which case main is currently wrong and week-in-merges should be deleted or re-indexed) or it is hand-curated (in which case the guard cannot derive removed from deleted files at all). Worth settling, because the guard's error message asserts the first and #4277 assumed the second.
Why this is not a block, given I blocked the structurally similar catalog-previews.yml finding on this same PR: there the break landed in a workflow with an opaque item not found exit, on someone editing renderer scripts with no reason to connect it to a catalog deletion. Here Catalog payloads is not a required context (the required set is Build, Render on windows-latest, Semantic PR title, Studio and player captures, Test, Test: runtime contract, Tests on windows-latest, Typecheck, regression), and the error names the offending item to someone who is editing the registry at that moment. It costs a minute, not an afternoon. It is still worth fixing before the next catalog addition, which on this repo's cadence is days away.
Non-blocking — two smaller notes on the guard
- The base anchor is read two different ways.
baseManifestcomes fromgit show origin/main:registry/registry.json— the tip of main — whiledeletedPathscomes fromorigin/main...HEAD, the merge base. Inert today only because main's item set is unchanged between24c3e197eand5ca2e7838. The moment main gains or drops an item while a PR is open, the guard attributes main's movement to that PR. Usinggit merge-basefor both, orbase + "...HEAD"semantics for both, removes the class. Concretely relevant here: #4276 is still open and touchesregistry/registry.json. - No test, where 8 of the 11 sibling
scripts/check-*.mjsguards have one (check-tracked-artifacts,check-no-main-deletions,check-test-reachability,check-workspace-contracts,check-package-cycles,check-pr-captures,check-docs-snippet-motion,check-cli-process-ownership). Not a rule —check-docs-catalogandcheck-skill-mirroralso lack one — but an added-item case in a test is exactly what would have caught the above.
Non-blocking — the fallow move left its rationale behind
The entry moved correctly, but the comment did not travel with it, and the destination is inside another group's rationale:
.fallowrc.jsonc:94still reads// Browser test intentionally contains repeated inline scenarios for distinct failure paths.— now sitting directly above"examples/**", which it does not describe.- At
:471the entry landed betweenpackages/aws-lambda/src/handler.tsandpackages/aws-lambda/src/s3Transport.ts, inside the four-line comment about AWS and GCP adapters deliberately mirroring each other. It splits a pair that comment addresses as a pair, and the browser test now reads as covered by a rationale about cloud adapters.
Moving the comment across with the entry, placed after s3Transport.ts, restores the file's convention — every exemption carrying its own reason is what made duplicates.ignore the right list in the first place.
Checked and fine
- The 25 removals are the whole delta. Zero additions, and no item changed
type. registry.jsonis the only manifest that moved in this commit.docs/public/catalog-index.json,docs/docs.jsonandregistry/catalog-artifact/local-vectors.{json,bin}are untouched byc83d8bdaand still carry the state I verified last round.- The guard is wired where it claims to be.
package.json:37chains it ahead oftsx scripts/catalog-drift.tsincheck:catalog-drift, whichci.yml:229runs in theCatalog payloadsjob. It runs on this PR and passes. - Every check at this head is green — no failing, no pending. Noting it as an observation; it is not what this verdict rests on.
Heads-up, unchanged from last round
#4276 is still open at 7a098147 and touches registry/registry.json and both catalog-artifact/local-vectors files. Whichever of #4274 / #4276 lands second needs its artifacts regenerated on the new base, and that regeneration is the step to verify rather than assume. With this guard in place the second one will now tell you if the regeneration was skipped — which is the point of it, and the reason the addition hole is worth closing first.
— Rames
Removes 29 catalog items with little or no use over the last month, keeping the three most-used variants of the code-snippet family.
Measured single removals stay below 150 uses and family tails keep the top three. Items with UNMEASURED usage remain untouched. No visible change to retained catalog items.