fix(ci): classify images/fonts/binaries as inert instead of production in revert-oracle

[BIMvoice]

Summary

classifyPath in the revert-oracle put every changed file it didn't recognize into production by default, including images, fonts, and other binaries with no observable behaviour. That made the oracle ABORT with "changes production code and adds/changes NO test file" on branches whose only changes were of this kind — no test can accompany or observe a deleted PNG.

Adds a narrow inert classification (images, fonts, and a few binary container formats — .png/.jpg/.svg/.ico/..., .woff/.woff2/.ttf/..., .zip/.gz/.tar/.pdf) alongside the existing production/test/ignored kinds. Extracted to scripts/lib/revert-oracle-inert.mjs to stay under scripts/lib/revert-oracle.mjs's frozen module-size budget (528 lines) rather than raising it.

.json and every other format a runner actually reads/executes stay production — a source file with no accompanying test still ABORTs exactly as before. Inertness is about the file kind, not the diff operation: a pure deletion of an inert file is inert too.

Test plan

• RED: added tests pinning images/fonts/binaries as inert (not production), that inertness survives a delete, that .json/.rs/.ts/.py production files are unaffected, and that a mixed production+inert diff still isolates the real production file — all 4 failed against the old classifier.
• GREEN: node --test scripts/lib/revert-oracle.test.mjs → 59/59 pass.
• Mutation: reverting the new inert branch in classifyPath and separately in classifyDiff's bucketing each independently drops exactly the 4 new tests to fail, confirming they're load-bearing.
• Sibling sweep: grepped every other revert-oracle-*.mjs module for file-kind classification logic — the classifier lives only in revert-oracle.mjs, no duplicate to fix.
• Gates: node scripts/check-module-size.mjs, node scripts/check-test-wiring.mjs, node scripts/check-source-text-assertions.mjs all exit 0. Full catch-all node --test scripts/*.test.mjs scripts/lib/*.test.mjs scripts/fixtures/*.test.mjs scripts/docs/*.test.mjs scripts/review/*.test.mjs scripts/review/lib/*.test.mjs → 2169/2169 pass.
• No changeset: scripts-only change, no published package touched.

Closes #4137

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 4 billable files and costs up to $1.00.}

Or wait 8 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: d6afdbac-0df6-4df8-84a5-3cf9b6f520f9

📥 Commits

Reviewing files that changed from the base of the PR and between bea417a and 32aac23.

📒 Files selected for processing (4)

• scripts/check-test-revert-oracle.mjs
• scripts/lib/revert-oracle-inert.mjs
• scripts/lib/revert-oracle.mjs
• scripts/lib/revert-oracle.test.mjs

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Claude review - no findings for 423b30138

Reviewed this diff and found nothing to flag.

[BIMvoice]

Adversarial review came back sound — I could not break it. Recording two things for the file.

The dangerous direction is clean today. Checked every listed suffix against the actual tree: git ls-files | grep -E '\.(pdf|zip|gz|tar|woff2?|ttf|otf|eot|bmp|tiff?|avif|webp|jpe?g|gif)$' returns empty, and no .wasm is tracked either. So nothing this list marks inert can currently swallow a real production change — which was the failure mode worth worrying about, since a false OBSERVED is worse than a noisy ABORT.

.svg is the loosest entry, and worth knowing about. apps/viewer/vite.config.ts feeds src/icons/*.svg through string-replace theming and svgo.optimize() at build time — real code operating on SVG bytes. Grepping the viewer's tests for anything asserting on that transform returns nothing, so no test is defeated by marking it inert today. But "no runner ever executes it" is not quite true for SVG, and if someone later snapshots the theming output using an SVG fixture, this entry would hide a regression. Not a defect now; the one line I would watch if the list grows.

The narrow direction was checked too — .ifc (367 files), .ids (388), .bcf (4), .ifcx (4), .parquet (3), .gltf (1) all correctly stay production, since each is genuinely parsed and asserted on. .snap files all live under __snapshots__ and are already caught by TEST_DIR_RE.

Verified: both halves mutation-tested independently (reverting classifyPath's branch → 4 failures; reverting classifyDiff's bucketing → 3), an all-inert or inert+test diff lands on the pre-existing prodPaths.length === 0 path (NOT APPLICABLE, exit 0) rather than an accidental verdict, case-insensitivity works (.PNG), .tar.gz matches on .gz, and revert-oracle.mjs sits at 523/528 with no allowlist edited.

---

Separately — a pre-existing gap this PR does not cause, but which is the same family as #4137:

packages/ids/src/__corpus__/ holds 341 of the 367 tracked .ifc files, and TEST_DIR_RE matches only __tests__|__snapshots__|__fixtures__|test-fixtures|testdata — not __corpus__. So a PR touching only IDS corpus fixtures classifies them as production and gets the same "changes production, no observing test" ABORT that #4137 exists to eliminate.

Worth its own issue if you agree — adding __corpus__ to TEST_DIR_RE looks like the obvious fix, but it is your call whether corpus fixtures count as test data or as inputs under test.

[BIMvoice]

Superseded by #4141 — suggest closing this one.

We collided on #4137. This PR opened at 05:38:35Z and #4141 at 05:48:41Z, so this was first by ten minutes, but yours is clearly the fuller treatment: +894/−97 across eight files with a dedicated revert-oracle-classify.mjs module and the cargo/python classifiers brought along, against +110/−14 here that only adds an inert bucket to classifyPath/classifyDiff.

Nothing here is worth salvaging separately as far as I can see — #4141 covers the same ground and more. Closing this avoids you reviewing two implementations of one issue.

For the record, in case any of it is useful while reviewing yours, this one was verified as follows:

• Suffix list checked in both directions. No .wasm, .pdf, .zip, font or listed-image file exists anywhere in the tree today (git ls-files | grep -E … → empty), so nothing marked inert could mask a real production change. In the other direction .ifc (367 files), .ids (388), .bcf, .ifcx, .parquet, .gltf all correctly stayed production, since each is genuinely parsed and asserted on.
• .svg is the loosest entry — apps/viewer/vite.config.ts runs string-replace theming and svgo.optimize() over SVG bytes at build time, so "no runner executes it" is not strictly true. No test currently observes that transform, so it is not a live risk, but it is the line I would watch if the list grows.
• All-inert and inert+test diffs land on the pre-existing prodPaths.length === 0 path (NOT APPLICABLE, exit 0) rather than an accidental verdict; case-insensitive matching works (.PNG), .tar.gz matches on .gz.

One thing found while reviewing it that is not in either PR and may be worth folding into #4141 or a follow-up: packages/ids/src/__corpus__/ holds 341 of the repo's 367 tracked .ifc files, and TEST_DIR_RE matches __tests__|__snapshots__|__fixtures__|test-fixtures|testdata but not __corpus__. A PR touching only IDS corpus fixtures therefore still gets the same "changes production, no observing test" abort that #4137 exists to remove. Whether corpus files count as test data or as inputs-under-test is your call.

[louistrue]

#4140 and #4141 both close #4137, and they take different approaches. This needs a decision, not a race.

First, my process failure: I filed #4137 and dispatched an agent against it without claiming the issue. AGENTS.md says to assign and comment before writing code, precisely so this does not happen. #4140 was opened at 05:38, before my #4141. That ordering is on me.

The two designs

#4140 classifies by an extension list: 20 suffixes (.png, .jpg, .svg, .woff2, .zip, .pdf, …) plus an exact-name set for lockfiles. Simple, readable, no dependency on git's behaviour.

#4141 derives inertness from a conjunction: the file is not a kind any runner compiles or executes, and git itself produced no textual diff for it. Unknown kinds fall out correctly without being enumerated.

Where each is stronger, stated fairly

#4140 is correct on the case that broke mine. .gitattributes marks apps/viewer/public/samples/**/*.ifc as -diff, so git reports those four text files as binary. My first implementation classified them inert, and about twenty Rust and CLI tests read them byte-for-byte. I needed a git check-attr guard to fix it. #4140 never consults git, so that hole cannot exist there: .ifc is simply not in the list.

#4141 is stronger on the kind that has not happened yet. A deny-list misses the next binary kind, and the miss renders as a finding naming the author. #4137's own text says not to write one, for that reason. Today the repo has no tracked .wasm, .mp4 or .glb; the day it does, #4140 aborts a PR that deletes one until someone edits the list. #4141 is silent about kinds nobody has thought of, which is the property #4109 argues for.

So the trade is: a hole that exists today and is fixed, versus a hole that opens whenever a new binary kind arrives. I do not think that is obviously my way. #4140's simplicity is a real virtue, and "no git dependency" removes an entire class of environment-sensitivity that mine carries (core.bigFileThreshold, attribute state, git version).

What I would suggest, and it is not "merge mine"

Land #4140 as the base, since it is first, simpler, and correct today. Then, if the unbounded-list concern is judged real, add the conjunction on top as a follow-up: keep the extension list as a fast path and let "no runner claims it AND git says it is bytes" catch the rest, with the check-attr guard already written and tested in #4141.

That way the repo gets the simple correct thing now and the general thing only if it earns its machinery.

If the preference is the other way round, #4141 is fully reviewed and green: seven end-to-end cases with verdict and exit code asserted, two mutation proofs (treating .rs as inert flips the abort; returning numstat alone fails the -diff case), and a blast-radius measurement of every tracked path (6,806 unspecified, 4 unset). Whichever loses should be closed with its useful parts folded into the winner rather than left open.

@louistrue this is your call. I am not merging #4141 over #4140.

Two pieces of #4141 are worth keeping regardless of which base wins:

1. The .gitattributes case as a test. Even a deny-list should have it pinned, since the next person to add a git-based condition will reintroduce the hole.
2. The note that -/- from git diff --numstat means "no textual diff", not "binary", and has three causes. That is the fact behind the bug.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated
ifc-lite-dev	Ignored		Sep 8, 2026 8:32am UTC
ifc-lite-viewer-embed	Ignored		Sep 8, 2026 8:32am UTC

[github-actions]

Claude review - no findings for 32aac23ca

Reviewed this diff and found nothing to flag.