Skip to content

fix(ts): TypeId snapshots alike on every host - #633

Merged
kixelated merged 3 commits into
mainfrom
quest/m0/typeid-snapshots
Nov 7, 2026
Merged

kixelated merged 3 commits into
mainfrom
quest/m0/typeid-snapshots

Conversation

@kixelated

@kixelated kixelated commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Since moq-dev/rust2#605, the lang/dyns and lang/panics/hook snapshots hold rustc's 128-bit TypeId hashes, and local just ci on main fails on hosts whose cargo config differs from the one that wrote them (reported in moq-dev/rust2#632's Follow-ups).

Cause, reproduced: not the checkout's path. One driver binary run from two checkouts at different paths gives the same -C metadata and byte-identical output. Cargo hashes the build profile into a crate's -C metadata, which seeds its StableCrateId and so the hash of every type the crate defines. This host's ~/.cargo/config.toml sets [profile.dev] debug = false. Translating with CARGO_PROFILE_DEV_DEBUG=true reproduces the committed snapshots byte for byte, and build.incremental changes the hash too. Std's types (u8, &str) hash alike everywhere.

Caused by

Approach

The snapshot comparison masks every TypeId, a 32-hex string literal, as "<TypeId N>", numbered by first appearance in its file. UPDATE_EXPECT writes the masked form, so a snapshot still shows which ids are equal. The translation still emits rustc's exact hash, so comparisons within a run, Debug's TypeId(0x…) shape, and the type_id differential tests are unchanged, and no differential assertion loosens.

Decisions

  • Decided with the maintainer on 2026-11-07: an XS m0 quest, started right away, planned and fixed in this PR. The quest file is added and deleted here, so its Plan lives in this section.
  • Mask in the snapshots rather than pin the harness's cargo profile. Every hashed profile field, any ancestor .cargo/config.toml, CARGO_PROFILE_* in the environment, and each nightly bump (the rustc version seeds StableCrateId too) would each need another pin, or churn these snapshots.
  • quest start refuses a quest that isn't on upstream main yet, so the claim is this PR from the quest's branch, quest/m0/typeid-snapshots.

Impact

  • No public API or wire change: test harness and snapshots only.

Alternatives

  • Pin each hashed profile field in the harness through CARGO_PROFILE_DEV_* env: fragile, as above.
  • Pin the fixtures' -C metadata through RUSTFLAGS: rustc mixes it with cargo's own value rather than replacing it, so the profile still leaks through.

Verification

  • With masking, the translation matches the snapshots under the host's profile, CARGO_PROFILE_DEV_DEBUG=true, and CARGO_BUILD_INCREMENTAL=true.
  • quest gate just ci passes in two checkouts at different paths.

Follow-ups

  • None.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5 with high effort)

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, low effort
  • Session: 01a1b4ee-1109-7f21-914c-ef4e73abf196, round 1
  • Head: 6f13789
  • Base: origin/main at efa6300
  • Score: 10/10, The diff only normalizes snapshot TypeId literals, preserving equality within each file and leaving generated code and differential execution unchanged; no actionable defects were found, though tests were not run in the read-only environment.
  • Findings: none

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

pullfrog Bot commented Nov 7, 2026

Copy link
Copy Markdown

Pullfrog  | Review this ➔

@kixelated
kixelated marked this pull request as ready for review November 7, 2026 07:35
@coderabbitai

coderabbitai Bot commented Nov 7, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 38 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9d7971b2-9a7b-417d-ab9b-59fe9bb82f0b
Commits

Reviewing files that changed from the base of the PR and between efa6300 and 6f13789.

Files selected for processing: 5

  • ts/fixtures/lang/dyns/expected/ids.ts
  • ts/fixtures/lang/dyns/expected/lib.ts
  • ts/fixtures/lang/panics/hook/expected/hooked.ts
  • ts/fixtures/lang/panics/hook/expected/lib.ts
  • ts/tests/fixtures.rs
  • 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

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed head 6f13789 against base efa6300.

No actionable findings. The normalization is confined to the in-memory snapshot comparison and UPDATE_EXPECT output. It does not rewrite the translated output directory consumed by typecheck or the differential harness, so runtime TypeId equality, ordering, hashing, and Debug behavior are still exercised with the original hashes. The four changed snapshots retain repeated/distinct identities within each file, matching the helper's per-file numbering. The checked 32-byte prefix also guards the subsequent string slicing.

Direction: snapshot normalization is a reasonable boundary for this host-dependent representation; keep the differential execution unmasked. The matcher intentionally treats any quoted 32-digit lowercase hex literal as an ID, and per-file numbering does not itself check cross-file identity, so these snapshots complement rather than replace runtime differential coverage.

Verification limits: GitHub-only static inspection of all five changed files and the snapshot/typecheck/differential call paths. No Rust tests, profile-variation experiments, builds, or CI runs executed; the reported portability results were not independently reproduced. Rechecked open/non-draft state, full head/base and existing reviews immediately before posting.

(Written by review (OpenAI))

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