{{ message }}
Repository navigation
fix: only fault attachments whose payload failed to resolve - #438
Merged
Merged
Conversation
Summary.validate() flagged every attachment whose content is RenderingContent.none. But an attachment with no payloadRef has no content by construction (Attachment.init never attempts an export), so a bundle containing payload-less attachments would exit 3 reporting degradation that did not occur — a false positive on the exact mechanism 3.0 asks users to trust. Extract the per-attachment decision into Attachment.failedToResolve, which only reports failure when a payload existed and produced no content. The tests build Attachments through the production initializer from the same legacy JSON shape XCResultKit decodes out of a bundle: one with no payloadRef (must not fault — reproduced the bug before the fix), one whose payload cannot be exported (must still fault). Fixes #387 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.
Fixes #387.
Problem
Summary.validate()flagged every attachment whose content isRenderingContent.none. But an attachment with nopayloadRefhas no content by construction —Attachment.initnever attempts an export when there is no payload to export. A bundle containing payload-less attachments would exit 3 reporting degradation that did not occur: a false positive on the exact mechanism 3.0 asks users to trust.Per the 4.0 design spec's guiding rule, a fault means a genuine failure to read something that should be there. An attachment that structurally has no payload is not a failure to export it.
Fix
The per-attachment decision moves into
Attachment.failedToResolve, which only reports failure when a payload existed (payloadId != nil) and resolving it produced no content.validate()now consults that predicate. Three files, +76/−2, no behavior change for any attachment that has a payload.Tests (written first, watched fail)
None of the generated fixtures contains a payload-less attachment (verified by walking all three bundles' legacy object graphs: 13 attachments, all with
payloadRef), so the reproduction buildsAttachmentthrough the production initializer from the same legacy JSON shape XCResultKit decodes out of a bundle:testAttachmentWithoutPayloadRefIsNotAResolutionFailure— nopayloadRef→ must not fault. Failed before the fix (reproducing the issue), passes after.testAttachmentWhosePayloadCannotBeExportedIsAResolutionFailure—payloadRefpresent but unexportable → must still fault. Passes before and after, pinning the true-positive path.Full suite on freshly generated fixtures: 30 tests, 0 failures (1 skipped, pre-existing).
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests