Repository navigation
fix: prevent partial ItemRef picture substitution - #494
Merged
Merged
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
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.
Closes #453.
An unlisted
ItemRef("image1")could selectBinData/image10.pngthrough the HWP5 writer's substring fallback and embed the wrong picture. Resolve unlisted ItemRefs using the same case-sensitive exact name, path-suffix and filename-stem rules asDocument::bin_stream_index. Preserve authoritative manifest lookup and the nativeBinRef::Idcompatibility branch. A missing match uses the existing content-freeBinaryAssetRemovedledger path.Pre-change compatibility measurement:
Five new regressions failed at their intended assertions on the test-only baseline
b5950a5: body/cell/Generic-list partial references lacked the asset-loss event; a later exact stem lost to an earlier partial name; writer/model resolution disagreed. Four existing ItemRef tests passed on that baseline. The tests also check emitted payload bytes or the absence of a synthesized stream.Focused verification at
c6d7f6d: HWP5 library tests 128/128 passed and all-target HWP5 clippy passed with-D warnings, Rust 1.93.0, two build/test workers. Existing assertions and fixture guards remain intact. The separate comment-only commit translates touched-file Korean prose as required by AGENTS.md; quoted sample identifiers/marker text and all non-comment source were preserved.Full verification at
03b0d6380c8a74e093db5215938bdbc3c7c0a7ae: exact-head remote Rust 1.93.0,CARGO_BUILD_JOBS=2 RUST_TEST_THREADS=2 scripts/check.sh </dev/null, with verifier stdin also closed, exit 0.The 49 missing-fixture skips include 4 optional cases; those absent fixture paths remain unverified. Public PDF parity auto-skipped because this environment lacks the pinned Poppler version; hosted CI reports that separate gate.