Skip to content

refactor(app): split persistence helpers - #694

Merged
Astro-Han merged 2 commits into
devfrom
codex/i687-persist-split
May 17, 2026
Merged

refactor(app): split persistence helpers#694
Astro-Han merged 2 commits into
devfrom
codex/i687-persist-split

Conversation

@Astro-Han

Copy link
Copy Markdown
Owner

Summary

Split persistence helper logic out of packages/app/src/utils/persist.ts into focused read/migration and localStorage resilience modules.

Why

#687 identifies persist.ts as a first-batch frontend infrastructure debt file because it mixed storage targets, cache/quota eviction, legacy migration, and the Solid persisted wrapper in one >500 LOC utility. This PR keeps the public facade in place while moving the easiest independent helper responsibilities behind clearer file boundaries.

Related Issue

Refs #687.

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

  • Confirm Persist, persisted, removePersisted, PersistTesting, and shouldDebugPersistedTerminalRead remain available from @/utils/persist.
  • Confirm cache/quota/eviction/fallback behavior is only moved into persist-local-storage.ts, not changed.
  • Confirm sync/async normalize and legacy migration behavior is only moved into persist-read.ts, not changed.
  • Confirm this PR does not touch serialize.ts or active UI rewrite lanes.

Risk Notes

Behavior risk is low but broad: persistence is shared by layout, terminal, settings, language, server, model, prompt, and other app state. The change is intended to be behavior-preserving. No persistence keys, namespaces, data migrations, UI behavior, or desktop storage APIs changed.

Remaining #687 scope intentionally deferred: serialize.ts remains the next independent first-batch file. #601/#602/#604/#605/#606/#595/#615 lanes are not touched.

How To Verify

Focused tests: bun test --preload ./happydom.ts ./src/utils/persist.test.ts ./src/pages/layout/layout-page-store.test.ts -> 24 pass, 0 fail
Typecheck: bun --cwd packages/app typecheck -> passed
Frontend inventory: bun run frontend:inventory -> productionOver500 19 -> 18; warn-only inventory still exits 0
Diff check: git diff --check -> no whitespace errors
Code review: Requesting Code Review pass found no P0-P2 issues; P3 test naming was fixed before commit

Screenshots or Recordings

Not required. This is a non-visual persistence helper extraction.

Checklist

  • Human review status is stated above as pending, approved, or not required
  • I linked the related issue, or stated why there is no issue
  • This PR has exactly one type label (bug, enhancement, task, or documentation), at least one primary routing label (app, ui, platform, harness, or ci), and exactly one priority label (P0 to P3), or I requested maintainer labeling
  • I described the review focus and any meaningful risks
  • I listed the relevant verification steps and the key result for each
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
  • I reviewed the final diff for unrelated changes and suspicious dependency changes
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English

@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 13 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a8a0e465-4b27-4745-b2b6-12a27add9637

📥 Commits

Reviewing files that changed from the base of the PR and between 6a91751 and 2e87ebc.

📒 Files selected for processing (4)
  • packages/app/src/utils/persist-local-storage.ts
  • packages/app/src/utils/persist-read.ts
  • packages/app/src/utils/persist.test.ts
  • packages/app/src/utils/persist.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/i687-persist-split

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the app Application behavior and product flows label May 17, 2026

@github-actions github-actions 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.

Suggested priority: P2 (includes user-path files (packages/app/src/utils/persist-local-storage.ts, packages/app/src/utils/persist-read.ts, packages/app/src/utils/persist.test.ts, packages/app/src/utils/persist.ts)).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@Astro-Han Astro-Han added task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work P2 Medium priority tech-debt Supplemental cleanup, maintainability, architecture, test, or quality debt context labels May 17, 2026

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request refactors the persistence logic by extracting local storage implementation and reading/migration utilities into dedicated files, persist-local-storage.ts and persist-read.ts. The changes introduce a caching layer with an eviction mechanism to manage storage quotas and include new tests for async data reading. Feedback was provided to optimize the key iteration in the eviction logic by replacing an unnecessary intermediate array with a standard for loop.

Comment thread packages/app/src/utils/persist-local-storage.ts Outdated
@github-actions

github-actions Bot commented May 17, 2026

Copy link
Copy Markdown

Perf delta summary

Comparator: pass

Profile / Scenario interaction median interaction worst long task max tbt frame gap p95 frame gap max jank count cls status
default / homepage-cold 40 -> 32 (-8) 56 -> 40 (-16) 91 -> 60 (-31) 41 -> 10 (-31) 16.8 -> 33.4 (+16.6) 133.3 -> 116.7 (-16.6) 4 -> 3 (-1) 0 -> 0 (0) pass
default / long-session-input-lag 48 -> 48 (0) 56 -> 48 (-8) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.7 (0) 16.8 -> 16.7 (-0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-streaming-long 56 -> 40 (-16) 72 -> 64 (-8) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 16.8 -> 33.4 (+16.6) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-call-expand 16 -> 16 (0) 16 -> 16 (0) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.8 (+0.1) 16.7 -> 16.8 (+0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-default-open-heavy-bash 32 -> 24 (-8) 32 -> 24 (-8) 63 -> 64 (+1) 13 -> 14 (+1) 33.4 -> 33.4 (0) 149.9 -> 166.6 (+16.7) 2 -> 1 (-1) 0 -> 0 (0) pass
default / terminal-side-panel-open 48 -> 48 (0) 56 -> 48 (-8) 0 -> 0 (0) 0 -> 0 (0) 33.3 -> 33.3 (0) 33.4 -> 33.4 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-scroll-reading 24 -> 16 (-8) 32 -> 24 (-8) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0.505 -> 0.505 (0) warn: cls

@Astro-Han
Astro-Han merged commit a4aac2e into dev May 17, 2026
28 checks passed
@Astro-Han
Astro-Han deleted the codex/i687-persist-split branch May 17, 2026 11:07
Astro-Han added a commit that referenced this pull request May 17, 2026
Prepare PawWork v2026.5.18 for the stable desktop release.

- Bump the desktop package version to 2026.5.18.
- Scope the diagnostics unreadable-file retention test to POSIX permission semantics so Windows advisory does not fail on chmod behavior that Windows does not enforce the same way.

Verification:
- Focused desktop diagnostics test passed locally: 11 pass / 0 fail.
- Release typecheck passed locally for packages/desktop-electron.
- PR #706 CI passed, including ci, desktop-smoke, e2e-artifacts, CodeQL, dependency-review, label-policy, commit-lint, and title lint.

Release notes:
- Drafted against the merged range since v2026.5.17: #691, #692, #693, #694, #702, and #703.
- Cold-read review completed before merge; wording was tightened to avoid overclaiming diagnostics impact and to keep verification short.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows P2 Medium priority task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work tech-debt Supplemental cleanup, maintainability, architecture, test, or quality debt context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant