Skip to content

[leas code] Share usage provider store lifecycle - #13558

Open
nwparker wants to merge 1 commit into
mainfrom
nwparker/leas-code-usage-store-lifecycle
Open

[leas code] Share usage provider store lifecycle#13558
nwparker wants to merge 1 commit into
mainfrom
nwparker/leas-code-usage-store-lifecycle

Conversation

@nwparker

Copy link
Copy Markdown
Contributor

Summary

  • Share cache loading, durable persistence, refresh gating, fingerprint invalidation, single-flight scans, and public scan-state projection across Claude, Codex, and OpenCode.
  • Share exact local-calendar range and worktree-fingerprint behavior.
  • Centralize duplicated durable-writer lifecycle tests while retaining provider-specific schema, scanner, serialization, pricing, scope, and attribution coverage.
  • Remove 249 repository lines overall, including 231 production lines.

Behavior is intended to remain unchanged.

Screenshots

No visual change.

Testing

  • pnpm lint (scoped lint and all changed-code gates passed)
  • pnpm typecheck
  • pnpm test (7 focused files / 45 tests passed)
  • pnpm build (pnpm run build:electron-vite passed)
  • Added or updated high-quality tests that would catch regressions

Additional gates passed:

  • Focused oxlint and oxfmt checks across all 13 changed files
  • pnpm run check:code-quality:changed -- origin/main
  • pnpm run check:reliability-gates
  • pnpm run check:max-lines-ratchet
  • git diff origin/main...HEAD --check

AI Review Report

Three independent reviews traced cache schema normalization, persistence failure behavior, scan concurrency, stale/fingerprint gates, provider adapters, and local/folder/SSH workspace handling. Review found and fixed a legacy Claude cache edge case where a null enabled value was not defaulted, plus a vacuous orphan-temp-file assertion in the centralized tests. A final review found no remaining P0/P1/P2 issue.

Cross-platform review covered macOS, Linux, and Windows path handling. The existing path utilities and local-worktree metadata loader remain authoritative; there are no shortcut, label, shell, or new Electron platform branches in this change. No mobile-facing or remote-wire surface changed.

Security Audit

No dependency, command execution, IPC, network, auth, or secret-handling surface changed. Cache paths and durable atomic writes continue through the existing path resolver and UsageCacheSnapshotWriter. Corrupt local analytics caches still fail closed to defaults, and scan persistence failures remain logged without turning successful scans into caller failures.

Notes

  • Product intent caveat: this is a behavior-preserving lifecycle extraction; provider-specific schema migration, pricing, scope, scanner ownership, and automation attribution intentionally remain separate.
  • The stale OOM branches origin/oom/11-b-providers-usage and origin/oom/16-b-codex should be restacked at the shared lifecycle persistence seam. Their bounded serialization work should retain UsageCacheSnapshotWriter, generation veto, durable rename, temp cleanup, and flush().
  • Existing unbounded cache growth is unchanged residual risk; this PR neither introduces nor expands it.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Added shared usage-calendar and worktree-fingerprint helpers. Added UsageProviderStoreLifecycle for cache loading, persistence, scan coordination, state management, and failure handling. Migrated Claude, Codex, and OpenCode stores to the shared lifecycle. Updated date filtering and constructor requirements. Simplified provider-store tests and added lifecycle, calendar, fingerprint, and compatibility coverage.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change, although the unrelated prefix adds minor noise.
Description check ✅ Passed The description includes all required sections and provides clear summaries of changes, testing, cross-platform review, security, and follow-up notes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/main/usage/usage-provider-store-lifecycle.ts (1)

88-101: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Compute the worktree fingerprint only when the freshness check needs it.

refresh computes the fingerprint before the gating check. Two costs follow:

  • On the fresh path, every read (getSummary, getDaily, getBreakdown, getRecentSessions each call refresh(false)) rebuilds the full worktree map and JSON-stringifies plus sorts every worktree, then discards the result.
  • On the force path, the value is never used, and runScan computes the same fingerprint again.

Move the computation inside the freshness branch.

♻️ Proposed refactor
   async refresh(force = false): Promise<PublicUsageProviderScanState<DataPresenceKey>> {
     if (!this.state.scanState.enabled) {
       return this.getScanState()
     }
-    const currentWorktreeFingerprint = await this.getCurrentWorktreeFingerprint()
     if (!force && this.state.scanState.lastScanCompletedAt) {
       const ageMs = Date.now() - this.state.scanState.lastScanCompletedAt
-      if (ageMs < STALE_MS && this.state.worktreeFingerprint === currentWorktreeFingerprint) {
+      if (ageMs < STALE_MS && this.state.worktreeFingerprint === this.getCurrentWorktreeFingerprint()) {
         return this.getScanState()
       }
     }

getCurrentWorktreeFingerprint performs no I/O, so it can also drop async.

src/main/claude-usage/store.ts (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Retire the max-lines disable directives if the refactor made them unnecessary. This PR moves cache loading, persistence, and scan orchestration out of both provider stores into UsageProviderStoreLifecycle. Both files still carry a file-level eslint-disable max-lines. The coding guidelines forbid this directive, so remove each one that the shrunken file no longer needs.

  • src/main/claude-usage/store.ts#L1-L1: measure the file against the configured max-lines limit and delete the directive if the file now complies.
  • src/main/opencode-usage/store.ts#L1-L1: apply the same check and delete the directive if the file now complies.

As per coding guidelines: "Never add eslint-disable max-lines or oxlint-disable max-lines, including line-specific variants."

Source: Coding guidelines


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 899a15f6-d713-42aa-9984-af5708c26209

📥 Commits

Reviewing files that changed from the base of the PR and between bbcf89e and a8d8c05.

📒 Files selected for processing (13)
  • src/main/claude-usage/store.test.ts
  • src/main/claude-usage/store.ts
  • src/main/codex-usage/store-snapshot.benchmark.test.ts
  • src/main/codex-usage/store.test.ts
  • src/main/codex-usage/store.ts
  • src/main/opencode-usage/store.test.ts
  • src/main/opencode-usage/store.ts
  • src/main/usage/usage-calendar-range.test.ts
  • src/main/usage/usage-calendar-range.ts
  • src/main/usage/usage-provider-store-lifecycle.test.ts
  • src/main/usage/usage-provider-store-lifecycle.ts
  • src/main/usage/usage-worktree-refs.test.ts
  • src/main/usage/usage-worktree-refs.ts

@@ -1,28 +1,24 @@
/* eslint-disable max-lines -- Why: this store owns Codex analytics persistence, scan policy, and renderer query semantics. Keeping them together prevents the Codex range/scope rules from drifting away from the scanner’s event model. */
/* eslint-disable max-lines -- Why: Codex pricing, range, scope, breakdown, and automation-attribution policies remain one cohesive store. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Remove the max-lines suppression.

Line 1 adds eslint-disable max-lines. The coding guidelines prohibit this suppression. Split CodexUsageStore at a concrete policy boundary until it meets the configured limit.

Source: Coding guidelines

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.

1 participant