ci(testing): differential suites gate goccia against bun - #1082
Merged
Conversation
Port the adversarial-findings differential harness to bun-native scripts/test-cli-differential.ts: each battery under scripts/differential/ runs under goccia-interpreted, goccia-bytecode, and bun (Vitest-semantics oracle), comparing mode parity on pass/fail counts and failed-test name sets against bun, with per-runtime timeouts counted as divergences by design — a hang is a finding. Batteries live outside tests/ so the main suite never executes them. Wired into the cli job of ci.yml and pr.yml after the other test-cli steps; green only once the fix layers below this one are in place. Documented in docs/differential-testing.md with a pointer row in docs/testing.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📝 WalkthroughWalkthroughAdds a differential CLI runner that compares Goccia interpreter, Goccia bytecode, and Bun. Adds syntax, module, built-in, matcher, and mock test batteries, CI execution, and testing documentation. ChangesDifferential testing lane
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant GocciaInterpreter
participant GocciaBytecode
participant Bun
CLI->>GocciaInterpreter: Run test battery
CLI->>GocciaBytecode: Run test battery
CLI->>Bun: Run test battery
GocciaInterpreter-->>CLI: Verdict and failed names
GocciaBytecode-->>CLI: Verdict and failed names
Bun-->>CLI: Verdict and failed names
CLI->>CLI: Report divergences and set exit status
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
frostney
marked this pull request as ready for review
August 5, 2026 21:30
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Owner
Author
|
@coderabbitai review |
Contributor
|
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
coderabbitai
Bot
added
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/differential/a-typesyntax.test.ts (1)
15-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace function declarations with arrow functions.
These GocciaScript tests and fixtures use prohibited
functiondeclarations. Use arrow functions. Defineinnerbeforeouterwhen conversion removes hoisting. Update battery labels that claim function-declaration or hoisting coverage.
scripts/differential/a-typesyntax.test.ts#L15-L25: convert the typed declarations to typed arrow functions.scripts/differential/b-modules.test.js#L15-L17: rename the default-export test after fixture conversion.scripts/differential/b-modules.test.js#L36-L42: rename the declaration and hoisting test after fixture conversion.scripts/differential/mods/circA.js#L5-L11: export arrow functions.scripts/differential/mods/circB.js#L5-L11: export arrow functions.scripts/differential/mods/klass.js#L19-L30: export arrow functions and defineinnerbeforeouter.scripts/differential/mods/live.js#L3-L11: export arrow functions.Based on learnings: do not use function declarations in tests and scripts JS files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/differential/a-typesyntax.test.ts` around lines 15 - 25, Replace the typed declarations unionReturn, intersectionReturn, and genericConstraint in scripts/differential/a-typesyntax.test.ts:15-25 with typed arrow functions. Also rename the affected battery labels in scripts/differential/b-modules.test.js:15-17 and :36-42 to remove function-declaration/hoisting claims; convert exported declarations to arrow functions in scripts/differential/mods/circA.js:5-11, circB.js:5-11, klass.js:19-30, and live.js:3-11, defining inner before outer in klass.js after conversion.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1315-1316: Pin Bun to the same explicit version in both
differential-testing jobs: add the matching bun-version configuration to the
setup-bun step associated with the differential test in .github/workflows/ci.yml
(lines 1315-1316) and .github/workflows/pr.yml (lines 778-779).
In `@docs/differential-testing.md`:
- Around line 53-55: Update docs/differential-testing.md lines 53-55 to qualify
the shared describe/test/expect statement as applying only to non-Goccia-only
batteries. Update docs/testing.md line 562 to explicitly state that Bun
execution and failed-name comparison exclude *.goccia.test.js batteries,
preserving the documented distinction between common three-runtime batteries and
Goccia-only batteries.
In `@scripts/test-cli-differential.ts`:
- Around line 141-155: Update the verdict comparison block to compare
failed-test names between interpreted Goccia and bytecode Goccia, not just
pass/fail counts. Also compare Bun’s passed and failed counts against both
Goccia verdicts, while preserving the existing runtime error handling and
detailed name-difference reporting.
---
Nitpick comments:
In `@scripts/differential/a-typesyntax.test.ts`:
- Around line 15-25: Replace the typed declarations unionReturn,
intersectionReturn, and genericConstraint in
scripts/differential/a-typesyntax.test.ts:15-25 with typed arrow functions. Also
rename the affected battery labels in
scripts/differential/b-modules.test.js:15-17 and :36-42 to remove
function-declaration/hoisting claims; convert exported declarations to arrow
functions in scripts/differential/mods/circA.js:5-11, circB.js:5-11,
klass.js:19-30, and live.js:3-11, defining inner before outer in klass.js after
conversion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 27568979-d65b-422e-930f-bcf9a7298bd7
📒 Files selected for processing (15)
.github/workflows/ci.yml.github/workflows/pr.ymldocs/differential-testing.mddocs/testing.mdscripts/differential/a-typesyntax.test.tsscripts/differential/b-modules.test.jsscripts/differential/c-builtins.test.jsscripts/differential/d-matchers.test.jsscripts/differential/e-mocks.goccia.test.jsscripts/differential/mods/circA.jsscripts/differential/mods/circB.jsscripts/differential/mods/klass.jsscripts/differential/mods/live.jsscripts/differential/mods/rex.jsscripts/test-cli-differential.ts
frostney
changed the title
frostney
added a commit
that referenced
this pull request
Aug 9, 2026
* fix(jsx): hex-escape non-ASCII bytes in scan-guard diagnostics CodeRabbit round 1, finding from #1075: the zero-progress guard embedded a single raw byte of a multi-byte UTF-8 attribute name, leaking an incomplete sequence into the CLI JSON envelope. Bytes >= #128 now render as \xNN and the offending char is captured once. The CLI regression pins the escaped form and that the whole message stays ASCII. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(bytecode): validate operand C for the object-member validation mode CodeRabbit round 1, finding from #1076: the .gbc loader never validated operand C when OP_VALIDATE_VALUE's mode selects VALIDATE_OP_REQUIRE_OBJECT_FOR_MEMBER (C carries KeyReg there; other modes use C as an immediate, so the generic register check excludes it). A mode-specific RequireRegister(C) now rejects malformed files at load time; new loader-validation unit tests prove the rejection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(testing): re-raise timeout errors from toThrow callables CodeRabbit round 1, findings from #1077: the generic Exception arm in ToThrow caught TGocciaTimeoutError, converting an expired test deadline into a matched thrown error so the deadline never unwound to ExecuteSuite — reproduced as a 100% false pass in both modes before the fix. An explicit re-raise arm now precedes the generic catch (the two lifecycle sites already had it; the describe-registration site correctly records a failure instead and is left alone). Also documents toThrow's asymmetric-matcher form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(testing): root path-walk intermediates and matcher arguments across getters CodeRabbit round 1, findings from #1078: toHaveProperty's path resolution held intermediate values unrooted while getters could run user code and reach GC safe points. Investigation showed the exposure is wider than reported — the matcher's own expected-value argument was collected by a GC triggered inside a getter (bytecode-mode crashes reproduced on three cases). Path intermediates, boxed containers, the expected value, and the resolved value are now nest-safe temp roots released in finally blocks; GC-forcing regressions cover all three crash shapes. Also tightens the toEqual doc summary to the precise undefined rules. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(parser): reject empty definite-assignment annotations, keep typeof queries whole CodeRabbit round 1, findings from #1079: 'let value!:;' and the var form were accepted because both branches only checked for the colon — an empty collected annotation now raises the annotations-required error in both. TypeOperatorExpectsOperand gains gttTypeof so a multiline typeof type query is not split by ASI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(runtime): close the abort-registration race, root event construction CodeRabbit round 1, findings from #1080: a signal aborting between the timeout check and abort-algorithm registration left a pending fetch that fired its abort event yet later settled normally — registration now happens before worker creation and a zero handle routes to the pre-flight already-aborted rejection. EventTarget/Event construction roots the new values across prototype resolution and option reads. ADR 0104 attributes the never-throw listener contract to WHATWG (propagation is this runtime's recorded deviation) and records the non-enumerable accessor convention, now pinned by descriptor tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(parser): CLI cases for empty definite-assignment annotations Companion to the #1079 review fixes: pins 'let value!:;' and 'var value!:;' rejection in both modes (hunk missed the batch commit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(coverage): merge loader worker-pool coverage and normalize its flags CodeRabbit round 1, findings from #1081: the script loader's two worker pool paths ran without enabling pool coverage or merging worker trackers, silently dropping hits under --coverage --jobs=N, and its Validate did not imply --coverage from --coverage-format/--coverage-output the way the test runner does. Both pool sites now mirror the runner (including the thread-init warm-up guard); jobs 1/2/4 verified identical on both paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): compare full differential verdicts and pin the bun oracle CodeRabbit round 1, findings from #1082: mode parity compared only pass/fail counts (a bytecode run failing a different test with the same count was a false pass) and bun comparison checked only failed-name sets from the interpreted run — failed-name sets now compare between goccia modes and bun's counts compare against both, with both detections proven via temporary divergence fixtures. The differential jobs pin bun-version 1.3.14 so the oracle cannot drift with latest, and the docs state the goccia-only battery exception consistently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: split type-annotations out of language.md The stack cumulatively grew docs/language.md to 1051 lines and docs/testing.md to 801, past the doc-length gate each layer respected individually. The Stage 1 type-annotations surface moves to docs/type-annotations.md (with executive summary, plus the type-syntax ASI notes that read better there), language.md keeps a stub and link per the no-duplication rule, a redundant testing.md sentence folds into the paragraph restating it, and indexes/cross-links are retargeted (ADR 0038 link maintenance only). No content trimmed; all four doc validators and markdownlint green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(review): round-2 polish on the fix layer CodeRabbit round 2 on #1085: the type-annotations doc no longer claims exact TypeScript extension parity (GocciaScript also recognizes JSX in .js/.mjs) and its combined example uses unique bindings so it runs when copied whole; the differential harness preserves bun's error value in the divergence line (a bun TIMEOUT now reports as such); the hex-escape regression asserts the complete lowercase \xNN escape rather than the prefix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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
scripts/test-cli-differential.ts(bun-native port of the handover's python harness) runs each battery inscripts/differential/under goccia-interpreted, goccia-bytecode, andbun test(Vitest-semantics oracle), comparing mode parity on pass/fail counts and failed-test name sets against bun. A per-runtime timeout counts as a divergence by design — a hang is a finding (that is how the JSX-transformer DoS was caught).tests/so the main suite never executes them (verified by file count);*.goccia.test.jsbatteries (goccia-only globals) skip bun and enforce only mode parity.clijob of ci.yml and pr.yml after the other test-cli steps — green only with the seven layers below merged, which is the point of the stack ordering. With this branch: all five batteries agree, 0 divergences, exit 0 — including battery A, which hung the 0.11.0 parser forever.Object.create(Array.prototype)strict-equality class-identity edge; iterator-snapshot GC exposure in Set/Map comparison; coverage path-key normalization; pr.yml redundant coverage matrix leg;docs/testing.mdnear its length limit.Part of the 0.11.0 adversarial-findings stacked-PR train (8 layers, each PR targets the layer below; merge bottom-up). Mini-spec: fix the differential-testing findings F1–F13/G4/G5 — JSX-transformer hang, toThrow semantics, Vitest matcher parity, coverage consistency, TS type-syntax gaps, AbortSignal events — and land the goccia-vs-bun differential battery harness as a CI gate, green at every layer.
Testing
docs/differential-testing.md(with executive summary) + pointer row indocs/testing.md🤖 Generated with Claude Code