Conversation
…torage for improved performance and clarity
✅ Deploy Preview for fast-logbook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
More reviews will be available in 32 minutes and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughこのPRは、アプリケーションの永続化層をlocalStorageからIndexedDBへ全面移行し、日付単位のログ管理とロールオーバー時刻機能を実装します。新しいストレージモジュールが非同期キー・バリュー操作とlocalStorageからの移行処理を提供し、すべてのモジュールがこれを使用するように再構成されています。日付選択UIと多言語対応が追加され、メインアプリケーションロジックはバッファード書き込み、日付別フィルタリング、BroadcastChannelによるクロスタブ同期を実装した大規模なリファクタリングを受けています。分析機能は削除されました。 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sw.js (1)
36-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
CACHE_NAMEの初期化が非同期レースになっており、precache が不定になります。
install時点でCACHE_NAMEが未設定の可能性があり、/js/lib/storage.jsを含むassetsの事前キャッシュが安定しません。install内で manifest 取得→CACHE_NAME決定→addAllを同一チェーンで待つ構成にしてください（またはビルド時に固定バージョンを埋め込む）。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sw.js` around lines 36 - 50, The CACHE_NAME initialization currently runs asynchronously outside the install handler causing a race; move the fetch('/manifest.json') and manifestData.version -> CACHE_NAME assignment into the self.addEventListener("install", ...) waitUntil promise chain so that you first fetch and set CACHE_NAME, then call caches.open(CACHE_NAME) and cache.addAll(assets) in the same chain; ensure the install handler’s e.waitUntil only resolves after manifest fetch, CACHE_NAME assignment, and cache.addAll complete (alternatively embed a fixed version at build time to avoid runtime fetch).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/rules/assistant-style.md:
- Line 1: The document header contains a misspelling "# Assistant stylle rules"
— change "stylle" to "style" so the heading reads "# Assistant style rules";
search the file for any other occurrences of "stylle" and correct them to
"style" (update the top-level heading and any rule titles or references that use
the misspelled word).
In `@index.html`:
- Line 171: ボタンの表示文言がハードコードされた "OK" になっているので、i18n と整合させるために
id="dateFeatureNoticeOkButton" を持つボタンの中身を直接書き換えず、既存の翻訳パターンに合わせて翻訳キー化してください;
具体的には該当ボタンを data-translate（またはプロジェクトで使われている同等の属性）を使って例えば
data-translate="common.ok" のような翻訳キーを紐付けるか、該当の初期化コードで dateFeatureNoticeOkButton の
innerText を翻訳関数でセットするように変更してください（ボタン id: dateFeatureNoticeOkButton を参照）。
- Around line 160-172: The modal can become unclosable because
dateFeatureNoticeModal is set with data-bs-backdrop="static" and
data-bs-keyboard="false" while the OK button (dateFeatureNoticeOkButton) lacks a
dismiss attribute and relies solely on js/main.js's setItem(...); fix by adding
a safe close path: add data-bs-dismiss="modal" to the button so Bootstrap can
always close it, and in js/main.js ensure the setItem(...) completion handler
explicitly hides the modal (use Bootstrap's Modal API on element with id
"dateFeatureNoticeModal") on both success and failure as a fallback.
In `@js/config.js`:
- Around line 73-77: The current selector assigns target based only on
ROUNDING_UNIT_MINUTE_KEY which causes the 'date-roll-over-time' key to match the
<h5> (data-translate) instead of the time <input>, so the UI input isn't
updated; modify the selection logic in the block that sets target (using key,
ROUNDING_UNIT_MINUTE_KEY and $$one) to special-case the 'date-roll-over-time'
key and select the input element (e.g. an
input[type="time"][data-translate='date-roll-over-time']) rather than the
generic [data-translate='...'] so target.value updates the correct element.
In `@js/lib/download.js`:
- Around line 112-115: downloadLog can pass null into generateFormattedLog
because getItem(LOG_DATA_KEY) or getItem(ROUNDING_UNIT_MINUTE_KEY) can return
null/undefined, causing .split() exceptions or NaN; fix by providing safe
fallbacks and type-coercion before calling generateFormattedLog: ensure logData
is set to a safe empty string when both the function argument log and
getItem(LOG_DATA_KEY) are falsy, ensure mins is coerced to a Number and falls
back to a sensible default (e.g., 1 or 0) when getItem(ROUNDING_UNIT_MINUTE_KEY)
is missing or NaN, and validate these values (logData as string, mins as finite
number) in downloadLog prior to invoking generateFormattedLog.
In `@js/lib/multilingualization.js`:
- Around line 27-31: The current placeholder logic uses window.__i18n_t(key)
(symbol: __i18n_t) which returns the key when untranslated, so the existing if
(translated) check doesn't detect missing translations; update the logic to
detect untranslated entries by checking the i18n dictionary existence (e.g.
window.__i18n_dict[key]) or by comparing translated === key, and if untranslated
fall back to '' before assigning elem.placeholder; adjust the branch handling
for elem.tagName === 'INPUT' || elem.tagName === 'TEXTAREA' to only set
placeholder when a real translation exists.
In `@js/main.js`:
- Around line 40-42: The current conversion const version = Number(stored ?? 0)
can produce NaN when stored contains a corrupt value, causing the migration
check to always fail; modify the logic around getItem(MIGRATION_VERSION_KEY) so
you coerce the retrieved stored value to a number and explicitly guard against
NaN (e.g., compute a numericValue from stored and set version to 0 when
Number.isNaN(numericValue)); update the code path that uses stored/version
(refer to MIGRATION_VERSION_KEY, getItem, stored, version) to use this NaN-safe
fallback so migrations are retried.
---
Outside diff comments:
In `@sw.js`:
- Around line 36-50: The CACHE_NAME initialization currently runs asynchronously
outside the install handler causing a race; move the fetch('/manifest.json') and
manifestData.version -> CACHE_NAME assignment into the
self.addEventListener("install", ...) waitUntil promise chain so that you first
fetch and set CACHE_NAME, then call caches.open(CACHE_NAME) and
cache.addAll(assets) in the same chain; ensure the install handler’s e.waitUntil
only resolves after manifest fetch, CACHE_NAME assignment, and cache.addAll
complete (alternatively embed a fixed version at build time to avoid runtime
fetch).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e3aa09a-8b51-4ee5-9abe-2c75c30cef42
⛔ Files ignored due to path filters (1)
manifest.jsonis excluded by!**/*.json
📒 Files selected for processing (11)
.claude/rules/assistant-style.md.npmrcconfig.htmlindex.htmljs/config.jsjs/lib/analytics.jsjs/lib/download.jsjs/lib/multilingualization.jsjs/lib/storage.jsjs/main.jssw.js
💤 Files with no reviewable changes (1)
- js/lib/analytics.js
1 participant
Summary
js/lib/storage.js). Data is written synchronously to a localStorage buffer on every keystroke and committed to IndexedDB only on date change, tab hide, or navigation — preventing data loss under debounce race conditions.runMigrations()system safely moves existing localStorage data to IndexedDB on first launch. Supports old key variants (date-roll-over-time-value). Migration is retried on failure.BroadcastChannel('fast-logbook-sync')replaces thestorageevent for real-time sync across tabs.initializedguard to preventsaveLogs()writing an empty buffer beforeloadLogs()completes (Bootstrap modal focus-move triggeredblur, corrupting IndexedDB). Fixedconfig.jsshortcut input placeholders not displaying. FixedtranslateElement()in i18n to setplaceholderinstead ofinnerHTMLfor<input>/<textarea>elements.