fix(project): preserve canonical raw-carrier data in filesystem saves (#553) - #802
Conversation
The Tauri filesystem save path previously serialized the typed StoryProject projection directly. That could discard opaque persisted fields and round unsafe numeric literals even though the canonical project boundary retained the original raw JSON carrier. Existing CURRENT files now pass through canonical admission and the shared owned-edit/writeback verifier before the atomic filesystem replacement. The autosave bridge accepts both Redux ProjectData and flat StoryProject inputs, and raw JSON compression preserves the admitted text instead of parsing and reserializing it. New-project creation remains unchanged; non-CURRENT sources still fail closed. The slice deliberately does not change storageService backend selection, add an IndexedDB fallback or dual-write, migrate legacy data, or switch Rust Core authority. The existing native filesystem-first path is hardened while its existing non-canonical fallback remains outside this writer. Technical refusal detail is retained on the error object but the public message is safe for UI callers. Validation: - git diff --check - pnpm exec biome check services/projectAutosaveEditBridge.ts services/fs/fsCore.ts services/fs/projectFsStore.ts tests/unit/services/fs/fsStores.test.ts - vitest run tests/unit/services/fs/fsStores.test.ts tests/unit/services/projectAutosaveEditBridge.test.ts --reporter=dot (131 tests passed)
The repository docs guard derives the Vitest test count from the current source set. Adding the filesystem raw-carrier regression increased that count from 8102 to 8103 while the 616-file count and all other metrics stayed unchanged. The generated README badge, technology table, directory-map note, and metrics section are synchronized through the existing metrics script; no product or test behavior was changed. Validation: - pnpm run sync:readme - git diff --check - cumulative PR budget: 5 files, 105 meaningful lines, 2 commits
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: qnbs/WorldScript-Studio/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 WalkthroughWalkthroughExisting CURRENT Tauri project files now use raw-text-preserving canonical writeback. The save path applies generation-checked owned edits and fails closed for unsupported sources. New files use direct creation. Tests cover these behaviors. ChangesCanonical project writeback
Priority: ➖ Normal Comment |
Reviewer's GuideExisting project saves now use admitted, generation-checked raw-carrier writeback instead of full reserialization, preserving opaque fields and exact numeric tokens while exposing a stable refusal error on unsafe writes; focused tests and repository test-count documentation are updated. Sequence diagram for admitted filesystem project savesequenceDiagram
participant Store as FsProjectStore
participant FS as Filesystem
participant Admission as admitCanonicalProjectDocument
participant Edit as buildAutosaveOwnedProjectEdit
participant Writeback as commitOwnedProjectEdit
Store->>FS: exists(projectFile)
alt new project file
Store->>FS: writeTextFileAtomic(compressData(projectToPersist))
else existing project file
Store->>FS: readTextFile(projectFile)
Store->>Admission: admitCanonicalProjectDocument(currentRaw, storedProjectSchema)
Admission-->>Store: canonical raw carrier
Store->>Edit: buildAutosaveOwnedProjectEdit(projectToPersist, currentRaw)
Store->>Writeback: commitOwnedProjectEdit(expectedGeneration, currentRaw, edit)
alt COMMITTED
Writeback-->>Store: updated raw carrier
Store->>FS: writeTextFileAtomic(compressJsonText(writeback.raw))
else refused
Writeback-->>Store: conflict or non-admitted result
Store-->>Store: ProjectCanonicalWritebackError
end
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
The PR reference guard requires the real GitHub PR number in the Unreleased release notes, which only exists after the draft PR is created. This metadata-only bootstrap correction records the bounded #553 filesystem raw-carrier save change as PR #802. It intentionally changes no source, tests, runtime behavior, or release version.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…on (#553) Root cause: The existing-project filesystem save path combined orchestration, canonical admission, raw-carrier writeback, and refusal-detail branching in saveProjectUnlocked. That structure obscured the preserve-first boundary and left the newly added fail-closed paths under-covered. Correction: Extract the existing CURRENT-project sequence into persistExistingCanonicalProject and resolve writeback refusal details through an explicit status helper. Add regression coverage for non-CURRENT admission and source-read failure, asserting the stable UI-safe error while proving the stored source remains unchanged. Synchronize README test metrics through the repository-owned script. Preserved invariants: The helper still reads and admits the stored raw carrier, requires CURRENT admission, computes the generation fence, applies only the owned autosave edit, requires COMMITTED, and atomically replaces the compressed raw carrier. Opaque fields, unknown nested data, exact unsafe numeric tokens, fail-closed refusal, marker ordering, and the public error boundary remain unchanged. Scope boundaries: New-project creation, StorageManager routing, IndexedDB fallback behavior, dual-write, legacy migration, Rust Core authority, Qt/R-15 work, and retry or recovery semantics are unchanged. CodeFactor/CodeScene structural findings are addressed only for saveProjectUnlocked; unrelated pre-existing complex methods remain follow-up/advisory. Validation: - Biome focused check: passed - Vitest fs/autosave focus: 133 tests passed - pnpm run typecheck: passed - git diff --check: passed - pnpm run pr:budget -- --base origin/main --prospective: OK (6 files, 164 meaningful lines, 4 commits) - cs delta --staged: saveProjectUnlocked Bumpy Road Ahead fixed; Code Health 3.90 -> 4.02 - PR_BUDGET_BASE=<verified PR base> pnpm run ci:prepush: passed
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
services/fs/projectFsStore.ts (1)
747-747: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftUse a conditional replacement when external edits are supported.
withLegacyRoutingOperationalready serializes in-processproject.jsonwriters through the productionFileSystemService.writeTextFileAtomiconly serializes local atomic writes and performs an unconditional rename. If another process changesproject.jsonafter the read at line 722, line 747 can replace that newer file and discard its opaque fields. Add a filesystem-level compare-and-replace based on the admitted source generation, or document that external modification is unsupported. An application lock alone cannot coordinate a separate process.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: qnbs/WorldScript-Studio/.coderabbit.yaml
Review profile: CHILL
Plan: Essentials
Run ID: f3d03312-f07b-44d6-b56d-0c6dcd03a650
📒 Files selected for processing (6)
CHANGELOG.mdREADME.mdservices/fs/fsCore.tsservices/fs/projectFsStore.tsservices/projectAutosaveEditBridge.tstests/unit/services/fs/fsStores.test.ts
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c7ce93b3d
ℹ️ 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".
Root cause: The canonical filesystem writer normalized read, admission, and commit refusals, but a failure during the final atomic replacement still escaped as a raw filesystem error. That broke the stable save-refusal boundary for callers even though the original source remained preserved. Correction: Wrap only the existing writeTextFileAtomic call in the canonical filesystem helper and retain the underlying technical failure as ProjectCanonicalWritebackError.detail. Add a regression test for atomic replacement failure and update the existing replacement-failure assertion to verify the stable public message plus technical detail. Synchronize the repository-owned README test metrics for the added test. Preserved invariants: The existing raw-carrier admission, CURRENT-only write authority, generation fence, owned-edit-only mutation, atomic replacement implementation, temporary-file cleanup, marker ordering, and preserve-first behavior are unchanged. The source file remains untouched when replacement fails. Scope boundaries: This is the single in-scope CodeAnt API-boundary correction for #553. It does not change StorageManager routing, IndexedDB fallback behavior, dual-write, migration, retry policy, Rust Core or Qt authority, recovery design, or any unrelated analyzer finding. Validation: - Focused Biome check and git diff --check passed. - Focused Vitest fs/autosave run passed: 134 tests. - PR_BUDGET_BASE=87dd85a3b9001770bc9838b65e9fc7f02d4d77c6 pnpm run ci:prepush passed. - Local CodeScene staged delta was attempted but unavailable because the CLI could not create a network socket in this environment; remote CodeScene remains the authoritative review gate.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5ff60b0b8
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Convert source-inspection failures to the stable refusal error. · projectFsStore.ts:836
services/fs/projectFsStore.ts:836
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winConvert source-inspection failures to the stable refusal error.
withLegacyRoutingOperationdoes not transform errors from its callback. Ifapis.exists(projectFile)rejects, the raw filesystem error reachessaveProjectcallers before either write path is selected. Catch this inspection failure and throwProjectCanonicalWritebackErrorwith the filesystem detail.
🧹 Nitpick comments (1)
services/fs/projectFsStore.ts (1)
836-836: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required QNBS-v3 rationale comment.
Document why absent files use direct creation while existing files use raw-carrier writeback. For example:
// QNBS-v3: create new sources directly; preserve admitted raw carriers only when replacing an existing source.As per coding guidelines, “Bei jeder inhaltlich relevanten Änderung in TypeScript oder JavaScript einen einzeiligen Kommentar im Format
// QNBS-v3: [...]ergänzen.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: qnbs/WorldScript-Studio/.coderabbit.yaml
Review profile: CHILL
Plan: Essentials
Run ID: 6bd02df7-425f-4a0d-bfa5-7d4a7fa2f88b
📒 Files selected for processing (3)
README.mdservices/fs/projectFsStore.tstests/unit/services/fs/fsStores.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.
#553) Root cause: - Full-snapshot autosave treated bootstrap-spread unknown top-level and entity properties as owned edits, so opaque values could be rounded or overwritten. - An omitted owned optional field produced no removal edit, allowing stale persisted state to reappear after reset/import. - Filesystem admission fenced only the initial read; an external replacement between readback and rename could be overwritten. Correction: - Whitelist declared ProjectData, Character, and World fields; merge only those fields over existing entities while preserving opaque raw members and all non-round-tripping numeric tokens. - Add preserve-first removal of omitted owned optional top-level fields, while keeping existing filesystem-only legacy-routing metadata explicit in the filesystem store. - Re-read and compare the admitted source generation immediately before each atomic filesystem rename, and fail closed through the stable ProjectCanonicalWritebackError boundary on a mismatch. Invariants and scope: - CURRENT-only admission, generation fencing, owned-edit verification, opaque-field survival, exact raw numeric tokens, atomic replacement, and stable UI-safe refusal text remain enforced. - No StorageManager routing change, IndexedDB dual-write, fallback removal, retry redesign, Rust Core authority switch, Qt/R-15 work, or unrelated tooling change. Validation: - Focused Vitest suites: 179 tests passed. - Changed-file Biome check, typecheck, and git diff --check passed. - README synchronized to 8111+ tests / 616 files. - PR budget OK: 8 files, 591 meaningful lines, 6 commits. - ci:prepush passed. - Local CodeScene review/delta unavailable because CLI 1.0.41 could not create a socket for license/telemetry in the constrained environment.
|
[check-pr-size] PR size is over the target tier (normal profile): 10 files, 1096 meaningful lines, 10 commits — limit ≤8 files / ≤400 lines / ≤6 commits. Consider splitting into smaller, independently reviewable PRs. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93e986df6e
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: qnbs/WorldScript-Studio/.coderabbit.yaml
Review profile: CHILL
Plan: Essentials
Run ID: 161e41de-f43c-4bfc-b697-346efdd9c185
📒 Files selected for processing (7)
README.mdservices/fs/fsCore.tsservices/fs/projectFsStore.tsservices/projectAutosaveEditBridge.tsservices/projectDocumentWriteback.tstests/unit/services/fs/fsStores.test.tstests/unit/services/projectAutosaveEditBridge.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- services/fs/projectFsStore.ts
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.
Root cause: the existing-project save path normalized read, admission, canonical writeback, and atomic replacement failures, but the project.json existence probe still ran outside the stable refusal boundary. A rejected probe could therefore expose a raw filesystem error before the save path selected direct creation or raw-carrier writeback. Correction: catch only the project-source existence inspection and rethrow ProjectCanonicalWritebackError with the technical filesystem detail. Add a regression proving the stable UI-safe message, project ID, diagnostic detail, and unchanged stored source. Record the non-obvious absent-file versus existing-file persistence boundary with one English QNBS-v3 rationale. Synchronize README metrics through the repository-owned script for the added test. Preserved invariants and scope: new-project creation remains direct; existing CURRENT sources still use admitted raw-carrier writeback, owned edits, generation fencing, verification, and atomic replacement. No StorageManager routing, fallback, dual-write, migration, retry, Rust Core, Qt, or R-15 behavior changed. Validation: focused Biome passed; focused fs tests passed with 121 tests; autosave/writeback tests passed with 59 tests; README synchronization and docs/release truth passed; PR budget passed at absolute limits with 8 files, 624 meaningful lines, and 7 commits before this commit; ci:prepush passed sequentially, including CSP, native readiness, Tauri plugin parity, QNBS-v3, and TypeScript single-checker admission.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c08d905f9f
ℹ️ 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".
…553) The exact-head review wave exposed remaining preserve-first gaps: omitted optional entity fields could survive a full-snapshot restore, parsed nested containers could round opaque numeric descendants, schemaVersion could be removed through the owned-edit API, and synchronous image admission yielded before rename. Canonical autosave edits now carry per-entity removal metadata, overlay known nested world records onto the admitted raw carrier, and reject schemaVersion removal. Filesystem admission keeps synchronous checks adjacent to atomic replacement while asynchronous source checks remain awaited. These changes preserve opaque fields, exact numeric tokens, generation fencing, fail-closed admission, and atomic replacement without adding fallback or dual-write behavior. The external-writer race between final filesystem revalidation and rename remains a native CAS/lock prerequisite and is intentionally not represented as solved by this bounded TypeScript slice. New-project creation, backend routing, Rust/Core authority, and R-15 remain unchanged. Validation: - pnpm exec biome check on all changed source/test files - focused Vitest: 62 project writeback/autosave tests, 26 fsCore tests, and 121 fsStores tests passed - pnpm run typecheck - pnpm run ci:prepush
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a3bc48baf
ℹ️ 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".
… success (#553) A full-snapshot autosave could replace owned top-level values with a typed projection, dropping opaque descendants or rewriting unchanged unsafe numeric tokens. The filesystem store also scheduled recovery snapshots before canonical admission and writeback had succeeded. Merge typed values over the admitted raw carrier recursively, match identity-bearing array entries by id, preserve unchanged RawNumberLiteral tokens, and use the preserve-first serializer for top-level overlays. Schedule the non-fatal auto-snapshot only after authoritative project replacement succeeds. Add regression coverage for opaque top-level descendants, unchanged unsafe numeric tokens, and refused-save snapshot isolation. CURRENT-only admission, generation fencing, owned-edit verification, stable refusal errors, and atomic replacement remain unchanged. The external cross-process CAS/lock prerequisite remains explicitly out of scope; new-project creation, backend routing, fallback behavior, Rust Core, Qt, and R-15 are unchanged. CodeScene test-duplication remains advisory follow-up rather than an analyzer-only source wave. Validation: - Biome and git diff --check - Vitest: projectAutosaveEditBridge 23/23, fsStores 122/122, projectDocumentWriteback 41/41, fsCore 26/26 - pnpm run typecheck - PR budget: 10 files, 1080 meaningful lines, 9 commits, status OK - PR_BUDGET_BASE=origin/main pnpm run ci:prepush
Root cause: The recursive raw-carrier overlay kept number, array, and object dispatch in one function. That introduced nested conditional complexity in a newly added persistence helper and made the preservation boundary harder to audit, even though the behavior was correct. Correction: Extract raw-record detection, id indexing, array-entry selection, array merging, and object merging into narrowly named helpers. The public dispatcher now only selects the existing number, array, object, or scalar behavior. Preserved invariants and scope: RawNumberLiteral tokens remain byte-exact when the typed numeric value is unchanged; id-bearing arrays still match by stable id with index fallback; object overlays still preserve opaque descendants and delete explicit undefined values. Canonical admission, CURRENT-only authority, generation fencing, owned-edit verification, stable refusal errors, and filesystem atomic replacement are unchanged. No test-duplication cleanup, external CAS/lock work, fallback/routing change, or broader #553 expansion is included. Validation: - pnpm exec biome check services/projectDocumentWriteback.ts - focused Vitest: projectDocumentWriteback and projectAutosaveEditBridge, 64 tests passed - pnpm run typecheck - local cs delta --staged: mergeRawCarrierValue Complex Method, Complex Conditional, and Bumpy Road Ahead fixed; Code Health 8.95 -> 10.00 - PR budget prospective: 10 files, 1096 meaningful lines, 10 commits, status OK - PR_BUDGET_BASE=origin/main pnpm run ci:prepush: passed
There was a problem hiding this comment.
Gates Failed
Prevent hotspot decline
(1 hotspot with Code Duplication)
Our agent can fix these. Install it.
Gates Passed
2 Quality Gates Passed
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| fsStores.test.ts | 1 rule in this hotspot | 8.28 → 7.79 | Suppress |
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
Purpose / root cause
Existing CURRENT Tauri filesystem saves previously serialized the typed StoryProject projection directly. That could discard opaque persisted fields and round unsafe numeric literals even though the canonical project boundary retained the original raw JSON carrier.
Exact implementation
PR #802 keeps new-project creation unchanged and routes existing project.json files through FsProjectStore.persistExistingCanonicalProject. The bounded path reads and decompresses the stored text, admits it as CURRENT canonical data, computes the source generation, builds only the owned autosave edit, commits through commitOwnedProjectEdit, requires a COMMITTED result, compresses the returned raw carrier, and performs the existing atomic filesystem replacement. The final structural correction extracts the raw-carrier array/object merge branches without changing their recursive semantics.
Invariants preserved
Explicitly out of scope
No StorageManager/backend-routing change, IndexedDB fallback removal, dual-write, legacy migration redesign, recovery/retry redesign, Rust Core or Qt authority switch, or R-15 implementation is included. The existing Tauri filesystem-first path and its non-canonical IndexedDB fallback boundary remain outside this writer slice. The external-writer check/rename race still requires a native cross-process CAS or lock and is an explicit follow-up; this PR does not claim to solve it. #553 is not being closed by this PR.
Regression proof
The focused filesystem, autosave-bridge, and writeback tests cover opaque top-level and nested descendants, exact unsafe and unchanged numeric tokens, owned-field removals, CURRENT admission, schemaVersion refusal, source-read/replacement refusal, generation fencing, and refused-save snapshot isolation. The current source and tests contain no typed JSON.parse-to-projection-to-JSON.stringify reconstruction of the canonical raw carrier.
Validation
Exact-head review dispositions
This is a bounded #553 filesystem raw-carrier writeback slice. It does not claim universal persistence parity, complete #553 acceptance, Rust Core observation-ledger completion, or native authority migration.