fix(ci): hash crates/engine/data and build.rs in engine-source-hash.sh#5461
Conversation
`scripts/engine-source-hash.sh` — the single authority for the parse-diff baseline key — hashed only crates/engine/src, Cargo.toml, and Cargo.lock, omitting crates/engine/data/** and crates/engine/build.rs. Both are compiled into the engine and change oracle-gen's parse output: oracle-subtypes.json is `include_str!`d at parser/oracle_util.rs, and known-tokens.toml is embedded via build.rs → OUT_DIR. So a PR touching only known-tokens.toml (or build.rs) produced an unchanged hash, and the "Parse-detail diff vs base baseline" step fetched a baseline built from a different parser state that happened to share the hash — under-reporting the PR's parse changes. Add both paths to the `git ls-tree` set so the fingerprint covers every input that determines parse output, matching ci.yml's cardgen-cache key (widened to include the same paths). Header comment updated to match. Verified: the script still emits a 16-hex digest, the two paths now contribute blobs (build.rs, known-tokens.toml, oracle-subtypes.json), and the digest differs from the old src+manifest+lock-only formula. Fixes phase-rs#5457.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the engine source hash script (scripts/engine-source-hash.sh) to include the engine data directory and build script in the fingerprint calculation. The review feedback points out a medium-severity issue where running the script from a subdirectory would fail to resolve paths correctly, and suggests adding the --full-tree flag to git ls-tree to resolve this.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @@ -37,6 +40,8 @@ sha="$1" | |||
| # Truncated to 16 hex chars to match the card_data_hash convention (ci.yml). | |||
| git ls-tree -r "$sha" -- \ | |||
There was a problem hiding this comment.
[MEDIUM] Use --full-tree to ensure pathspecs are resolved relative to the repository root.
Why it matters: Without --full-tree, running this script from any subdirectory (e.g., scripts/) causes git ls-tree to resolve the paths relative to that subdirectory, resulting in no matches and silently producing the hash of an empty input.
Suggested fix: Add --full-tree to the git ls-tree command.
| git ls-tree -r "$sha" -- \ | |
| git ls-tree --full-tree -r "$sha" -- \ |
|
Reviewed the current head and addressed the reviewer finding: |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Reviewed the current head and addressed the reviewer finding: git ls-tree now uses --full-tree, so the hash is stable when this helper is invoked from a subdirectory as well as the repository root. Local verification produced the same digest and 652 matching entries from both working directories. The change is scoped to the CI hash helper and is ready for the merge queue.
2 participants
Fixes #5457.
Problem
scripts/engine-source-hash.shis the single authority for the parse-diff baseline key (computed on both the publish and fetch sides of the coverage-parse-diff flow). It hashed onlycrates/engine/src,crates/engine/Cargo.toml, andCargo.lock— omittingcrates/engine/data/**andcrates/engine/build.rs, both of which are compiled into the engine and change oracle-gen's parse output:crates/engine/data/oracle-subtypes.jsonisinclude_str!d into the parser (parser/oracle_util.rs).crates/engine/data/known-tokens.tomlis embedded viabuild.rs→OUT_DIR.Consequence: a PR touching only
known-tokens.toml(orbuild.rs) yields an unchanged engine-source hash, so the "Parse-detail diff vs base baseline" CI step fetchesparse-baselines/coverage-data-<hash>.jsonbuilt from a different parser state that happens to share the hash, and under-reports the PR's parse changes. Same failure family as theci.ymlcache-key gap already fixed — different file.Fix
Add
crates/engine/dataandcrates/engine/build.rsto thegit ls-treepath set, and update the header comment. This mirrorsci.yml'scardgen-cache key, which was already widened to hashcrates/engine/data/**andcrates/engine/build.rs(ci.yml lines 215/243).Per the issue, changing the hash function orphans existing
coverage-data-<hash>.jsonobjects for one main-push cycle (PRs see "Baseline pending" until main republishes) — benign and self-healing, same as any parser-state change.Verification (local)
git ls-tree -r HEAD -- crates/engine/data crates/engine/build.rsnow contributes the three engine-compiled blobs (build.rs,known-tokens.toml,oracle-subtypes.json).src + Cargo.toml + Cargo.lock-only formula, confirming those inputs now actually participate — so aknown-tokens.toml/build.rs-only PR will now invalidate the baseline key.ci.yml(which invoke the script and consume its output); no test hardcodes the old path set or digest.