Skip to content

Fix Codable scene storage read coherence - #6

Open
stalkermv wants to merge 3 commits into
mainfrom
fix/scene-storage-codable
Open

stalkermv wants to merge 3 commits into
mainfrom
fix/scene-storage-codable

Conversation

@stalkermv

@stalkermv stalkermv commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner

After a local write, SceneStorage.Codable could keep returning its cached value after a peer wrapper or native scene-storage writer changed the bytes. Read native Data on every property and binding access, and reuse only successful decodes whose key and complete bytes still match.

Keep the memo in a local StateObject without publishing cache mutations. Successful assignments clear the local decoded memo even when bytes are unchanged. Bindings forward their transaction. Invalid JSON returns the declared default; encoding failures preserve both stored bytes and the memo and emit a fixed diagnostic. Standard JSON null remains distinct from a missing value.

Compatibility

Value and projected-binding access now require the main actor. Initializers, JSON encoding, typed-key identity and deployment declarations retain their existing forms. Reference results may retain identity between reads; assignment is required to persist mutations. AppStorage behavior is unchanged.

Validation

  • Library unit tests: swift test --build-system native passed 94 tests across 15 suites using Swift 6.4.2, including regression coverage for successful same-byte reference reassignment and rejected-write cache preservation. The new reassignment test failed before the fix.
  • Fixture API coverage now belongs to the executable target: macOS and iOS Simulator builds explicitly compiled APIForms.swift. Root swift test does not build or launch this nested app; its build/runner is separate.
  • Installed-scene runtime checks below precede the successful-write cache-invalidation follow-up; they were not rerun for that patch.
  • Actual-product macOS and iPad peer/native/ABA/onChange scenarios: 52/52 each. The same fixture against the old implementation produced 24 expected wrapper failures with zero native-control failures on each platform.
  • Extended scene fixtures: 41/41 macOS, 40/40 iPad, including two real WindowGroup scenes, failure byte preservation, cache lifetime, identity reconstruction and dynamic key/default parity.
  • Deployment-floor module/API compilation: macOS 12 and iOS 14 rechecked after the review fix; watchOS 9 and Catalyst 14 passed before that follow-up. Exact Swift 6.1 is untested; tvOS verification is excluded from this cycle.
  • Relaunch compares wrapper/native behavior; restored-value branches were unobserved, so no persistence durability claim is added.
  • A prior run exposed an intermittent demand failure in unchanged TaskFuture/Combine tests. The current full run passed; this PR does not fix TaskFuture.

README documents actor access, fallback/write errors, JSON null, reference caching, key identity and native hosting limitations. The standalone fixture README documents its runtime suites and reproduction commands.

Read native scene bytes on every access and memoize only matching
successful decodes. Preserve binding transactions and fall back to the
declared default when decoding fails.

BREAKING CHANGE: CodableSceneStorage value and binding access require
the main actor. Decode failures return the declared default rather than
a prior successful value.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T02:06:00.396269Z 9fc897a PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9fc897a022

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Tests/Fixtures/SceneStorageProbe/Package.swift
Include reference-payload and nonisolated initialization checks in
every fixture build. Clarify that root swift test does not run the
standalone scene application.
@stalkermv stalkermv self-assigned this Oct 18, 2026
@stalkermv
stalkermv requested a balanced review from Copilot October 18, 2026 02:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Successful assignments can retain stale reference values, and the transaction-test helper lacks a required availability annotation.

2 open issues
What changed in this PR

Updates Codable scene storage to consult native bytes on every read while caching successful decodes. AppStorage behavior remains unchanged.

Changes:

  • Adds a scene-local cache and transaction-preserving bindings.
  • Adds unit tests and installed-scene regression fixtures.
  • Documents actor access, fallback behavior and storage limitations.
File Description
Tests/​SwiftStorageTests/​CodableStorageTests.swift Removes unhosted scene-storage assertions.
Tests/​SwiftStorageTests/​CodableSceneStorageTests.swift Tests caching, failures and transactions.
Tests/​Fixtures/​SceneStorageProbe/​Sources/​SceneStorageProbe/​Probe.swift Adds peer/native coherence scenarios.
Tests/​Fixtures/​SceneStorageProbe/​Sources/​SceneStorageProbe/​LifecycleProbe.swift Exercises cache lifetime and dynamic keys.
Tests/​Fixtures/​SceneStorageProbe/​Sources/​SceneStorageProbe/​HostingAndRestoration.swift Checks hosting and restoration parity.
Tests/​Fixtures/​SceneStorageProbe/​Sources/​SceneStorageProbe/​FullProbe.swift Coordinates extended and multiscene checks.
Tests/​Fixtures/​SceneStorageProbe/​Sources/​SceneStorageProbe/​FailureProbe.swift Checks failures and JSON formats.
Tests/​Fixtures/​SceneStorageProbe/​Sources/​SceneStorageProbe/​APIForms.swift Provides public API compile coverage.
Tests/​Fixtures/​SceneStorageProbe/​run_probe.sh Launches the Python runner.
Tests/​Fixtures/​SceneStorageProbe/​run_probe.py Builds, launches and records fixture results.
Tests/​Fixtures/​SceneStorageProbe/​README.md Documents fixture execution and scope.
Tests/​Fixtures/​SceneStorageProbe/​Package.swift Defines the standalone fixture package.
Sources/​SwiftStorage/​CodableStorage.swift Removes the previous scene-storage implementation.
Sources/​SwiftStorage/​CodableSceneStorage.swift Implements native-byte reads and decode caching.
README.md Documents updated scene-storage semantics.

Review effort: Balanced

Comment thread Sources/SwiftStorage/CodableSceneStorage.swift Outdated
Comment thread Tests/SwiftStorageTests/CodableSceneStorageTests.swift
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.

2 participants