fix(ci): treat __corpus__ and sibling fixture dirs as test data in revert-oracle - #4145
Merged
Conversation
…n revert-oracle TEST_DIR_RE did not match __corpus__, so a branch touching only packages/ids/src/__corpus__/ (334 of the repo's 367 tracked .ifc files) was classified as changing production code with no accompanying test and false-aborted. #4142 presented two options without a maintainer choice in the issue's comments (there were none): treat __corpus__ as test data, or leave it production and require revert-oracle-exempt per PR. This implements the first, inferred rather than chosen for me -- it matches how sibling directories are already treated. The trade-off accepted: a corpus edit that genuinely changes behaviour no longer gets flagged as needing an observing test. Option 2 (revert-oracle-exempt per PR) remains available if that trade-off turns out wrong. A repo-wide sweep for directories in the same shape (raw fixture data, not code) added __test__, bare corpus (fuzz seed inputs under rust/*/fuzz/corpus) and test-data alongside __corpus__. Left out as genuine production, verified by mutation: packages/collab/src/snapshot/ (a feature), packages/source-fixture/ and packages/world-frame-fixtures/ (published packages with their own src/), scripts/fixtures/*.mjs (tooling scripts), and the app samples/ directories (assets shipped to users). Refs #4109
Contributor
Claude review - no findings for
|
github-actions
Bot
added
the
llm-reviewed
|
Warning Review limit reached
This review includes 2 billable files and costs up to $0.50. Or wait 59 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe revert oracle now classifies ChangesFixture classification
Priority: ➖ Normal — Impact reflects medium issue severity. Estimated code review effort: 1 (Trivial) | ~5 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Fixture and corpus directories now avoid false production-without-tests aborts while similarly named production and asset paths retain production classification. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
github-actions
Bot
removed
the
llm-reviewed
BIMvoice
added a commit
that referenced
this pull request
Sep 8, 2026
…t inert #4137's new `inert` bucket demotes a binary path with no runner claiming it as source. The four `.bcf` fixtures under `packages/bcf/test-data/` are real zip archives read byte-for-byte by reader.test.ts, schema-validation.test.ts and the fabricated-*.test.ts suites, but `test-data` matched neither TEST_DIR_RE (testdata, test-fixtures) nor TEST_SEGMENT_RE (needs a literal test/tests segment). On this branch they classified `production` before #4137 and silently drop to `inert` under it -- exactly the silent-miss direction the bucket exists to avoid. Verified against #4145 (open, not yet merged): that PR separately adds `test-data` (plus `__corpus__`/`corpus`/`__test__`) to the pre-split revert-oracle.mjs, but #4137 already moved TEST_DIR_RE out of that file into revert-oracle-classify.mjs, so #4145's patch cannot land here as-is. The regression exists on this branch today independent of merge order, so the fix belongs here. Swept the repo's fixture-directory naming conventions for the same shape by enumerating every binary path in the tree and classifying it: the only other binaries hitting `inert` are icons/favicons (correctly inert -- no test reads them) and rust/geometry/fuzz/corpus/** seeds (correctly inert -- only `cargo fuzz run` touches them). apps/landing/samples/hello-wall.parquet also lands `inert`; no test references it (only the sibling .ifc, which stays production via the .gitattributes `-diff` exemption), so that is a real shipped-but-unwitnessed asset, not a regression. `packages/extensions/src/ testing/` was checked and confirmed to stay `production` (exports public SDK API), so it was deliberately left out of the pattern.
Contributor
Claude review - no findings for
|
github-actions
Bot
added
the
llm-reviewed
github-actions
Bot
removed
the
llm-reviewed
Contributor
Claude review - no findings for
|
github-actions
Bot
added
the
llm-reviewed
github-actions
Bot
removed
the
llm-reviewed
Contributor
Claude review - no findings for
|
github-actions
Bot
added
the
llm-reviewed
6 tasks
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
2 participants
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
TEST_DIR_REinscripts/lib/revert-oracle.mjsdid not match__corpus__,so a branch touching only
packages/ids/src/__corpus__/-- 334 of therepo's 367 tracked
.ifcfiles -- was classified as production code withno accompanying test, and the oracle false-aborted:
Same false-abort family as #4137 and #4108, tracked under the umbrella
#4109.
The choice
#4142 presented two options without advocating one. I checked the issue's
comments and there were none, so no choice was made for me. This PR infers
option 1 -- treat
__corpus__as test data -- rather than presenting it asthe only possibility:
__corpus__(and the siblings found by the sweep below) toTEST_DIR_RE. This matches how__fixtures__,test-fixturesetc. arealready treated, and is what the revert-oracle renders capability gaps as findings about the PR (umbrella: #4050, #4085, #4104, #4107, #4108) #4109 umbrella is converging on. Trade-off
accepted: a corpus edit that genuinely changes behaviour no longer gets
flagged by the oracle as needing an observing test.
__corpus__classified as production and requirerevert-oracle-exempton every corpus-only PR. Keeps the signal at thecost of a manual label each time. This option remains available if the
trade-off above turns out wrong for a maintainer.
Sibling sweep
Enumerated every directory segment in the repo (
git ls-files, grouped bybasename) that looks fixture-like, and checked each against
TEST_DIR_RE/TEST_SEGMENT_RE:packages/ids/src/__corpus__/.ifc/data fixtures, the issue's subjectapps/viewer/src/lib/__test__/__test__, singular) --stubs.tssays "Production code never imports from this module", imported only by*.test.tsrust/{core,geometry}/fuzz/corpus/corpus) -- raw fuzzer seed inputs, no codepackages/bcf/test-data/test-data, hyphenated) --.bcffixtures imported only by*.test.tspackages/{bcf,export,server-client}/src/__fixtures__/,packages/ids/src/audit/__fixtures__/__fixtures__)*/tests/{fixtures,golden,snapshots,census_golden}/(rust crates,packages/provenance/test/golden,packages/extensions/test/fixtures)tests?/segment)packages/collab/src/snapshot/packages/source-fixture/,packages/world-frame-fixtures/src/, versioned, changelogsscripts/fixtures/(build-manifest.mjs,fetch-fixtures.mjs,upload-fixtures.mjs)packages/data/scripts/fixtures/epsg-control-points.jsonapps/landing/samples/,apps/viewer/public/samples/examples/,rust/*/examples/,demo/Bare
fixturesis deliberately not added, precisely becausescripts/fixtures/*.mjsholds real tooling scripts -- adding it would havemisclassified production code as test scaffolding.
Verification
RED, before the fix:
GREEN, after:
Over-reach check (mutation-style, run against every look-alike found by the
sweep): all still classify
production, e.g.packages/collab/src/snapshot/from-step.ts->production,packages/source-fixture/src/index.ts->production,scripts/fixtures/build-manifest.mjs->production,apps/viewer/public/samples/hello-wall.ifc->production. Genuineproduction paths (
packages/ids/src/audit/engine.ts,rust/core/src/parser.rs)still classify
productiontoo.Mutation of the fix itself: reverting
TEST_DIR_REto the original fivealternatives drops the suite from 56/56 to 55 pass / 1 fail -- exactly the
new sibling test added for #4142.
node --test scripts/lib/revert-oracle.test.mjs: 56 pass, 0 fail(was 54 before this change; +2 test blocks).
Also ran and confirmed exit 0:
check-module-size.mjs(revert-oracle.mjsstays at its exact 528-line budget -- the doc comment above
TEST_DIR_REwas kept to one line for this, with the fuller rationale in
revert-oracle.test.mjsinstead),check-test-wiring.mjs,check-source-text-assertions.mjs.Conflict with the revert-oracle cluster
#4140, #4141 and #4131 all edit
scripts/lib/revert-oracle.mjsin or nearthis same region (classification helpers /
TEST_DIR_REneighbourhood).#4141 in particular restructures classification into a new
revert-oracle-classify.mjs. This PR's diff torevert-oracle.mjsis a2-line change (one comment, one regex), so it should be low-friction to
rebase onto whichever of #4140/#4141/#4131 merges first, but it will likely
need to sequence behind them rather than merge independently.
No changeset: scripts-only change to internal CI tooling, no published
package changes -- following the precedent of the other revert-oracle
fixes in this cluster, none of which carry a changeset.
Closes #4142
Summary by CodeRabbit
Bug Fixes
Tests