Skip to content

refactor(hermes): fold duplicated base/raw/transform logic, update v99 docs - #99

Merged
sunnylqm merged 6 commits into
masterfrom
claude/hermes-cleanup
Oct 12, 2026
Merged

sunnylqm merged 6 commits into
masterfrom
claude/hermes-cleanup

Conversation

@sunnylqm

@sunnylqm sunnylqm commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Step 4 of the cleanup: the Hermes modules (hermes-base, hermes-raw, hermes-literals, hbcTransform) and their docs. No behavior change is intended. Every change is checked against the full local HBC matrix (see Verification).

Changes

  • hbcTransform
    • The late-v98 and v99 layout entries were identical apart from the version, so they become one 98..99 entry.
    • The early-v98 header is now derived as the late header minus numStringSwitchImms.
    • tryTransformPair and tryTransformPairFiles share the same-version check and the meta builder.
    • The chosen candidates and the wire layout sent with patches are byte-identical for every version 80..100 (compared as JSON before and after).
  • hermes-raw
    • Reuses the layout resolveHbcSections already picked instead of running the same search again via pickLayout.
    • One local helper replaces three copies of the normalizeRawHermesFunction call.
    • Exports foldWidthSuffix and isFunctionHeader for the pretty pass.
  • compareHermesBytecode reads each HBC file once. The literal buffers are views into the bytes already loaded for the semantic data, instead of reopening the file for the header and three sections.
  • hermes-base
    • classifyHermesCommand: one result instead of three identical return blocks.
    • resolveHermesBase: drop its never-read hermesCommand param; compute source after the null check; pick the artifact type once; handle a cache hit in one branch.
    • fetchBaseBundle: the "server ignores Range" fallbacks share rangeIgnored() / fromArchive().
    • One isBundleHash() replaces the four cache-name regexes.
  • docs
    • hermes-base-verification.md now covers HBC v99: the verified range, the shaped-layout tables, the four-job CI matrix, the v99 function header, and the undecoded NewTypedObjectWithBuffer.
    • Step 5.3 now keys off the detail text, since the literals field is not logged.
    • The one lasting rule from the PR fix(hermes-base): stop rejecting a base over a printed property name #85 review log (keep the pretty pass) is folded in, and hermes-review-follow-up.md is removed.

Deliberately not changed

  • The text-only fallback path. Dropping it is a product decision: unsupported layouts would report dump-failed instead of rejected, and the fake-hermesc harness relies on it.
  • The two dump line splitters. Only the raw one strips CR. Merging them would change the pretty pass on CRLF output, which no test or CI job covers.

Verification

  • bun run lint passes; bun test: 584 pass, 32 skip, 0 fail.

  • Local copy of the CI hermes-integration matrix (same pinned compilers and Metro fixtures with sha256 checked), with tests/hermes-*.test.ts plus 50 seeded fuzz rounds each:

    HBC compiler tests fuzz
    96 react-native 0.77.3 143 pass, 1 skip exit 0, 5/5 planted caught
    98 hermes-compiler 250829098.0.16 143 pass, 1 skip exit 0, 4/4 caught
    98 hermes-compiler 250829098.0.17 143 pass, 1 skip exit 0, 4/4 caught
    99 hermes-compiler 260318099.0.4 142 pass, 2 skip exit 0, 5/5 caught

    The counts match a master baseline taken the same way.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Documentation
    • Updated Hermes verification guidance to cover v99 compiler validation, function-header layouts, and CI coverage.
    • Added troubleshooting guidance for buffer comparison differences and potential overlap false positives.
  • Bug Fixes
    • Improved fallback handling when range requests are unsupported, including reuse of responses already received.
    • Ensured cached compiler artifacts with unknown bytecode versions are checked against the requested version before selection.
    • Improved in-memory HBC analysis and buffer comparisons across supported layouts.

sunnylqm and others added 6 commits October 12, 2026 15:05
- classifyHermesCommand: the build / commitly / stable version forms share
  one result instead of three identical return blocks
- resolveHermesBase never read its hermesCommand parameter; drop it
- hermes-base: one isBundleHash() for the four cache-name checks
- hbcTransform: tryTransformPair and tryTransformPairFiles share the
  same-version check and the meta builder; the late-v98 and v99 layouts
  were identical apart from the version and become one 98..99 entry, and
  the early-v98 header is the late one minus numStringSwitchImms. The
  candidates and wire layout for every version 80..100 are byte-identical
  to before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
readHermesSemanticData ran the same first-fitting-layout search twice
(resolveHbcSections, then pickLayout on the same bytes); resolveHbcSections
now returns the layout it chose and pickLayout is internal again. The raw
dump splitter calls normalizeRawHermesFunction through one local helper
instead of three copies of the same call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- the three "server ignores Range" catches share rangeIgnored(), and the
  two full-archive endings share fromArchive()
- resolveHermesBase computes `source` after the null-record check, picks
  the artifact type once instead of per attempt, and checks a cache hit in
  one branch

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- hermes-base-verification.md: v99 (hermes-compiler 260318099.0.4) in the
  verified range, the shaped-layout tables and the CI matrix (four jobs);
  a note on the v99 function header and the undecoded
  NewTypedObjectWithBuffer; the `literals` result field is not logged, so
  step 5.3 now keys off the detail text
- fold the one lasting rule of hermes-review-follow-up.md (keep the pretty
  pass: raw does not cover exception-handler tables) into it and drop that
  PR #85 review log
- hermes-base pointed at a design doc that is not in the repo; v98-only
  comments now say v98 and v99

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
compareHermesBytecode read both files whole for the semantic data, then
reopened them to read the header and three literal sections again. The
literal buffers are now views into the bytes already in memory
(literalBuffersFromBytes / hermesSemanticData); the file-based
readLiteralBuffers and readHermesSemanticData stay as thin wrappers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The pretty (hermes-base) and raw (hermes-raw) dump readers each had their
own operand-width suffix fold and function-header regex; hermes-raw now
exports both. The two line splitters stay separate: only the raw one strips
CR, and unifying them would change the pretty pass on CRLF output, which no
test or CI job exercises.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b9e48a15-4810-4f0b-8291-5fa9c88e6197

📥 Commits

Reviewing files that changed from the base of the PR and between 14dc34c and d766779.


📒 Files selected for processing (8)
  • docs/hermes-base-verification.md
  • docs/hermes-review-follow-up.md
  • src/bundle-runner.ts
  • src/utils/hbcTransform.ts
  • src/utils/hermes-base.ts
  • src/utils/hermes-literals.ts
  • src/utils/hermes-raw.ts
  • tests/hermes-base.test.ts

💤 Files with no reviewable changes (3)
  • src/bundle-runner.ts
  • docs/hermes-review-follow-up.md
  • tests/hermes-base.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.



📝 Walkthrough

Walkthrough

The changes add HBC v99 layout coverage, expose in-memory parsing helpers, and update paired transforms and Hermes base resolution. Base comparison now reads each file once. Verification documentation adds v99 details, and a follow-up document is removed.

Changes

Hermes bytecode handling

Layer / File(s) Summary
HBC layouts and byte parsing
src/utils/hbcTransform.ts, src/utils/hermes-literals.ts, src/utils/hermes-raw.ts
Layout resolution returns the selected HBC layout. Literal-buffer and semantic-data parsers accept in-memory bytes. Shared helpers identify function headers and fold opcode width suffixes.
Paired HBC transforms
src/utils/hbcTransform.ts
Buffer and file-pair transforms use shared version validation and metadata construction, while still validating candidate layouts for both inputs.
Base resolution and archive fallback
src/utils/hermes-base.ts, src/bundle-runner.ts, tests/hermes-base.test.ts
Base resolution centralizes bundle-hash checks and archive extraction. Ignored Range responses use the received archive. Artifact source classification follows server-record validation, and the hermesCommand argument is removed from the call and test parameters.
Bytecode comparison and verification
src/utils/hermes-base.ts, docs/hermes-base-verification.md, docs/hermes-review-follow-up.md
Equivalence comparison reads each HBC file once and derives literal and semantic data from those bytes. Verification documentation adds v99 coverage and troubleshooting details. The follow-up document is deleted.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor


Merge Risk: ⚪ Minimal · up to d7667

No actionable issue remains from this review. The change is mergeable after normal checks.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 76.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 4 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title accurately identifies the main refactoring work and the v99 documentation update. It is concise and specific.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 76.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 4 files. (1 skipped: 1 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR


🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@sunnylqm
sunnylqm merged commit b86803b into master Oct 12, 2026
9 checks passed
@sunnylqm
sunnylqm deleted the claude/hermes-cleanup branch October 12, 2026 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant