feat: Phase 28 Code Hygiene & Logging - #382
Merged
Merged
Conversation
Create lib/logger.ts with level-filtered output (debug/info/warn/error). In production, only warn and error are emitted. Replace all 124 raw console.* calls across 28 web app source files with logger equivalents. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All .catch with empty callbacks on IPFS unpin calls and AudioContext close calls now log warnings via the structured logger so failures are visible instead of silently swallowed. Also fixes the one occurrence in packages/sdk/src/client.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace all `as any` casts in production web code with typed alternatives. Polyfill globalThis augmentations use proper declare-global blocks, the debug error log uses a typed Window interface, and the Zustand debug store uses a typed window property. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove 00-Preliminary-R&D/poc/ which was superseded by the production implementation. The POC code is preserved in git history. Add ARCHIVED.md documenting what was removed and where to find it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Create plans, summaries, and verification for Phase 28. All 4 success criteria verified: structured logger, visible unpin failures, no as-any casts, POC archived. Update ROADMAP.md and STATE.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 26 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
WalkthroughPhase 28: Code Hygiene & Logging is marked complete with implementation of a structured logger, replacement of 124+ console calls with logger.* calls across the web app, conversion of silent Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
5 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #382 +/- ##
=======================================
Coverage 60.61% 60.62%
=======================================
Files 128 128
Lines 9524 9526 +2
Branches 925 925
=======================================
+ Hits 5773 5775 +2
Misses 3540 3540
Partials 211 211
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
FSM1
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/web/src/components/file-browser/BinBrowser.tsx (1)
214-214: Include caught error objects in bin action logs.Current logs lose stack/cause context, which makes production debugging harder.
Proposed refactor
- } catch { - logger.error('[Bin] Restore failed'); + } catch (error) { + logger.error('[Bin] Restore failed:', error); }- } catch { - logger.error('[Bin] Permanent delete failed'); + } catch (error) { + logger.error('[Bin] Permanent delete failed:', error); }- } catch { - logger.error('[Bin] Batch restore failed'); + } catch (error) { + logger.error('[Bin] Batch restore failed:', error); }- } catch { - logger.error('[Bin] Batch permanent delete failed'); + } catch (error) { + logger.error('[Bin] Batch permanent delete failed:', error); }- } catch { - logger.error('[Bin] Empty bin failed'); + } catch (error) { + logger.error('[Bin] Empty bin failed:', error); }Also applies to: 235-235, 263-263, 281-281, 301-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/file-browser/BinBrowser.tsx` at line 214, The bin action error logs (e.g., the logger.error('[Bin] Restore failed') calls in the BinBrowser component) drop the caught Error object and lose stack/cause context; update each bin action catch block (restore, delete, permanent delete/empty handlers in BinBrowser.tsx) to include the caught error by logging a descriptive message plus the error object (e.g., logger.error('[Bin] Restore failed', error) or logger.error(`[Bin] Restore failed: ${error.message}`, error)) so the full error/stack is captured for debugging.apps/web/src/hooks/useBin.ts (1)
59-61: Consider capturing the error object for debugging.The catch handler doesn't capture the rejection error, so the logged message provides no diagnostic details. Adding the error parameter would improve observability.
♻️ Suggested improvement
void purgeExpired({ retentionDays: currentRetention, userPublicKey: auth.vaultKeypair.publicKey, userPrivateKey: auth.vaultKeypair.privateKey, - }).catch(() => { - logger.error('[useBin] Auto-purge failed (non-blocking)'); + }).catch((err) => { + logger.error('[useBin] Auto-purge failed (non-blocking):', err); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/hooks/useBin.ts` around lines 59 - 61, The catch handler in useBin's auto-purge promise swallows the error; update the .catch callback to accept an error parameter (e.g., err) and pass it into the logger.error call so the message includes the actual error details (reference the .catch(...) on the auto-purge promise and the logger.error('[useBin] Auto-purge failed (non-blocking)') call) to improve observability.apps/web/src/hooks/useAuth.ts (1)
164-164:logger.info()is silenced in production — consider if this diagnostic is needed.This "new user initializing vault" message uses
logger.info(), which is filtered out in production whereminLevelisLogLevel.WARN. If this diagnostic is valuable for troubleshooting production onboarding issues, consider usinglogger.warn()instead, or accept that it's development-only instrumentation.Per
apps/web/src/lib/logger.tslines 33-37, production only emits WARN and above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/hooks/useAuth.ts` at line 164, The log call in useAuth.ts currently uses logger.info('[Auth] New user -- initializing vault'), which is filtered out in production; change this to logger.warn('[Auth] New user -- initializing vault') if you want this diagnostic to appear in production (or explicitly document/leave it as info if it should remain development-only). Update the call in the useAuth hook where that exact message is logged to use logger.warn so it will be emitted when minLevel is LogLevel.WARN.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/components/file-browser/BinBrowser.tsx`:
- Line 214: The bin action error logs (e.g., the logger.error('[Bin] Restore
failed') calls in the BinBrowser component) drop the caught Error object and
lose stack/cause context; update each bin action catch block (restore, delete,
permanent delete/empty handlers in BinBrowser.tsx) to include the caught error
by logging a descriptive message plus the error object (e.g.,
logger.error('[Bin] Restore failed', error) or logger.error(`[Bin] Restore
failed: ${error.message}`, error)) so the full error/stack is captured for
debugging.
In `@apps/web/src/hooks/useAuth.ts`:
- Line 164: The log call in useAuth.ts currently uses logger.info('[Auth] New
user -- initializing vault'), which is filtered out in production; change this
to logger.warn('[Auth] New user -- initializing vault') if you want this
diagnostic to appear in production (or explicitly document/leave it as info if
it should remain development-only). Update the call in the useAuth hook where
that exact message is logged to use logger.warn so it will be emitted when
minLevel is LogLevel.WARN.
In `@apps/web/src/hooks/useBin.ts`:
- Around line 59-61: The catch handler in useBin's auto-purge promise swallows
the error; update the .catch callback to accept an error parameter (e.g., err)
and pass it into the logger.error call so the message includes the actual error
details (reference the .catch(...) on the auto-purge promise and the
logger.error('[useBin] Auto-purge failed (non-blocking)') call) to improve
observability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0b50e087-07bd-4504-a865-a31ebc842976
⛔ Files ignored due to path filters (1)
00-Preliminary-R&D/poc/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (53)
.planning/ROADMAP.md.planning/STATE.md.planning/phases/28-code-hygiene-logging/28-01-PLAN.md.planning/phases/28-code-hygiene-logging/28-01-SUMMARY.md.planning/phases/28-code-hygiene-logging/28-02-PLAN.md.planning/phases/28-code-hygiene-logging/28-02-SUMMARY.md.planning/phases/28-code-hygiene-logging/28-03-PLAN.md.planning/phases/28-code-hygiene-logging/28-03-SUMMARY.md.planning/phases/28-code-hygiene-logging/28-04-PLAN.md.planning/phases/28-code-hygiene-logging/28-04-SUMMARY.md.planning/phases/28-code-hygiene-logging/28-VERIFICATION.md00-Preliminary-R&D/ARCHIVED.md00-Preliminary-R&D/poc/.env.example00-Preliminary-R&D/poc/README.md00-Preliminary-R&D/poc/package.json00-Preliminary-R&D/poc/scripts/gen-private-key.ts00-Preliminary-R&D/poc/src/index.ts00-Preliminary-R&D/poc/state/state.json00-Preliminary-R&D/poc/tsconfig.jsonapps/web/src/components/file-browser/AudioPlayerDialog.tsxapps/web/src/components/file-browser/BinBrowser.tsxapps/web/src/components/file-browser/FileBrowser.tsxapps/web/src/components/file-browser/InviteLinkTab.tsxapps/web/src/components/file-browser/PdfPreviewDialog.tsxapps/web/src/components/file-browser/ReplaceFileDialog.tsxapps/web/src/components/file-browser/ShareDialog.tsxapps/web/src/components/settings/ConnectionTest.tsxapps/web/src/hooks/useAuth.tsapps/web/src/hooks/useBin.tsapps/web/src/hooks/useDropUpload.tsapps/web/src/hooks/useFileDownload.tsapps/web/src/hooks/useFileOperations.tsapps/web/src/hooks/useFileVersions.tsapps/web/src/hooks/useFolderNavigation.tsapps/web/src/hooks/useSearch.tsapps/web/src/hooks/useSharedNavigation.tsapps/web/src/lib/crypto/key-wrapping.tsapps/web/src/lib/logger.tsapps/web/src/lib/sw-registration.tsapps/web/src/lib/web3auth/core-kit-provider.tsxapps/web/src/lib/web3auth/hooks.tsapps/web/src/main.tsxapps/web/src/polyfills.tsapps/web/src/services/bin.service.tsapps/web/src/services/delete.service.tsapps/web/src/services/device-registry.service.tsapps/web/src/services/folder.service.tsapps/web/src/services/ipns.service.tsapps/web/src/services/search-index.service.tsapps/web/src/services/share.service.tsapps/web/src/services/upload.service.tsapps/web/src/stores/folder.store.tspackages/sdk/src/client.ts
💤 Files with no reviewable changes (7)
- 00-Preliminary-R&D/poc/scripts/gen-private-key.ts
- 00-Preliminary-R&D/poc/tsconfig.json
- 00-Preliminary-R&D/poc/state/state.json
- 00-Preliminary-R&D/poc/.env.example
- 00-Preliminary-R&D/poc/package.json
- 00-Preliminary-R&D/poc/README.md
- 00-Preliminary-R&D/poc/src/index.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Implements Phase 28 “Code Hygiene & Logging” by introducing a structured web logger, making previously-silenced async failures visible, tightening TypeScript typings for global/window shims, and removing an obsolete PoC directory (with archival documentation).
Changes:
- Added
apps/web/src/lib/logger.tsand replaced most web-appconsole.*usage withlogger.*calls. - Replaced silent
.catch(() => {})patterns (notably around IPFS unpin and AudioContext close) with warning logs. - Removed
as anycasts viadeclare globaltype augmentations; archived/removes00-Preliminary-R&D/poc/and updates planning docs/state.
Reviewed changes
Copilot reviewed 53 out of 54 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/sdk/src/client.ts | Logs SDK unpin best-effort failures instead of silently swallowing them. |
| apps/web/src/stores/folder.store.ts | Adds typed Window.__ZUSTAND_FOLDER_STORE__ for DEV/E2E access without as any. |
| apps/web/src/services/upload.service.ts | Switches upload failure logging to structured logger.error. |
| apps/web/src/services/share.service.ts | Replaces console warnings with logger.warn for share rotation/rewrap operations. |
| apps/web/src/services/search-index.service.ts | Uses logger.warn for non-blocking IndexedDB persistence/load failures. |
| apps/web/src/services/ipns.service.ts | Uses logger.warn for skipped signature verification scenarios. |
| apps/web/src/services/folder.service.ts | Replaces console logs with structured logger for sync/publish/rotation warnings & errors. |
| apps/web/src/services/device-registry.service.ts | Uses structured error logging while keeping registry failures non-blocking. |
| apps/web/src/services/delete.service.ts | Uses logger.error when per-file deletes fail. |
| apps/web/src/services/bin.service.ts | Makes bin/cleanup/unpin failures visible and routes logs through logger. |
| apps/web/src/polyfills.ts | Replaces as any global shims with declare global augmentation and globalThis assignment. |
| apps/web/src/main.tsx | Types DEV-only window error capture without as any. |
| apps/web/src/lib/web3auth/hooks.ts | Replaces console logging with logger.* for CoreKit auth flow traces/errors. |
| apps/web/src/lib/web3auth/core-kit-provider.tsx | Uses logger.error for CoreKit init failures. |
| apps/web/src/lib/sw-registration.ts | Uses logger for SW support/registration warnings & errors. |
| apps/web/src/lib/logger.ts | New structured logger wrapper with level filtering. |
| apps/web/src/lib/crypto/key-wrapping.ts | Uses logger.error for key re-wrap failures while continuing traversal. |
| apps/web/src/hooks/useSharedNavigation.ts | Routes shared navigation errors through structured logger. |
| apps/web/src/hooks/useSearch.ts | Logs index persistence/init/clear issues via logger. |
| apps/web/src/hooks/useFolderNavigation.ts | Uses logger.error for folder navigation/load failures. |
| apps/web/src/hooks/useFileVersions.ts | Makes unpin failures visible via logger.warn. |
| apps/web/src/hooks/useFileOperations.ts | Makes unpin failures visible and routes warnings via logger. |
| apps/web/src/hooks/useFileDownload.ts | Uses logger.error for download failures. |
| apps/web/src/hooks/useDropUpload.ts | Logs cleanup module-load and unpin failures instead of silently swallowing. |
| apps/web/src/hooks/useBin.ts | Logs non-blocking auto-purge failures via logger.error. |
| apps/web/src/hooks/useAuth.ts | Routes auth/vault init failures and non-blocking init errors through logger. |
| apps/web/src/components/settings/ConnectionTest.tsx | Uses logger.warn for fallback path messaging. |
| apps/web/src/components/file-browser/ShareDialog.tsx | Uses logger.error for share dialog failures. |
| apps/web/src/components/file-browser/ReplaceFileDialog.tsx | Logs replace/unpin failures instead of silent catches. |
| apps/web/src/components/file-browser/PdfPreviewDialog.tsx | Logs PDF load/render failures via logger.error. |
| apps/web/src/components/file-browser/InviteLinkTab.tsx | Logs invite fetch/create/revoke failures via logger.error. |
| apps/web/src/components/file-browser/FileBrowser.tsx | Uses structured logger for sync/move/download/delete/create failures. |
| apps/web/src/components/file-browser/BinBrowser.tsx | Uses logger.error instead of console for restore/delete batch failures. |
| apps/web/src/components/file-browser/AudioPlayerDialog.tsx | Logs AudioContext close/setup failures via logger.warn. |
| 00-Preliminary-R&D/poc/yarn.lock | Deletes legacy PoC lockfile as part of archiving/removal. |
| 00-Preliminary-R&D/poc/tsconfig.json | Deletes legacy PoC TS config as part of archiving/removal. |
| 00-Preliminary-R&D/poc/state/state.json | Deletes legacy PoC state file (contained sensitive-looking material). |
| 00-Preliminary-R&D/poc/src/index.ts | Deletes legacy PoC harness implementation. |
| 00-Preliminary-R&D/poc/scripts/gen-private-key.ts | Deletes legacy PoC key generation script. |
| 00-Preliminary-R&D/poc/package.json | Deletes legacy PoC package manifest. |
| 00-Preliminary-R&D/poc/README.md | Deletes legacy PoC documentation. |
| 00-Preliminary-R&D/poc/.env.example | Deletes legacy PoC env template. |
| 00-Preliminary-R&D/ARCHIVED.md | Adds provenance note documenting PoC removal and where to find it in history. |
| .planning/phases/28-code-hygiene-logging/28-VERIFICATION.md | Adds Phase 28 verification record. |
| .planning/phases/28-code-hygiene-logging/28-04-SUMMARY.md | Adds Plan 28-04 summary (archive PoC). |
| .planning/phases/28-code-hygiene-logging/28-04-PLAN.md | Adds Plan 28-04 plan document. |
| .planning/phases/28-code-hygiene-logging/28-03-SUMMARY.md | Adds Plan 28-03 summary (remove as any). |
| .planning/phases/28-code-hygiene-logging/28-03-PLAN.md | Adds Plan 28-03 plan document. |
| .planning/phases/28-code-hygiene-logging/28-02-SUMMARY.md | Adds Plan 28-02 summary (log unpin failures). |
| .planning/phases/28-code-hygiene-logging/28-02-PLAN.md | Adds Plan 28-02 plan document. |
| .planning/phases/28-code-hygiene-logging/28-01-SUMMARY.md | Adds Plan 28-01 summary (logger + replacements). |
| .planning/phases/28-code-hygiene-logging/28-01-PLAN.md | Adds Plan 28-01 plan document. |
| .planning/STATE.md | Advances planning state to Phase 28 complete / Phase 29 focus. |
| .planning/ROADMAP.md | Marks Phase 28 complete and lists completed plans. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
FSM1
enabled auto-merge (squash)
March 28, 2026 04:15
Merged
FSM1
added a commit
that referenced
this pull request
Mar 28, 2026
Resolve conflicts from Phase 28 squash merge (#382): - Take main's logger module prefixes for all web app files - Keep Phase 29 TODO cleanup in folder.service.ts - Keep Phase 29 SDK unenrollment additions + prettier fix in client.ts - Re-apply privacy fix (remove file.name from batch download log) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 28, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 28: Code Hygiene & Logging
Goal: Production web app uses structured logging instead of raw console.* calls, unpin failures are visible, type safety gaps are closed, and legacy POC is archived
Status: Verified ✓
Created a structured logger module, replaced all raw console calls across the web app, made IPFS unpin failures visible, eliminated
as anycasts, and archived the legacy POC directory.Changes
Plan 28-01: Structured Logger & Console Replacement
Created
apps/web/src/lib/logger.tswith LogLevel enum (DEBUG/INFO/WARN/ERROR/SILENT), level filtering (production emits WARN+ only), and timestamped structured output. Replaced all 124console.*calls across 28 web app source files.Key files:
apps/web/src/lib/logger.ts(new), 28 files modified across services/, hooks/, components/, lib/Plan 28-02: Fix Silenced Unpin Failures
Replaced all 15
.catch(() => {})patterns with.catch((err) => logger.warn(...))so IPFS unpin failures are visible in logs. Also fixed the SDK client occurrence.Key files:
bin.service.ts,ReplaceFileDialog.tsx,AudioPlayerDialog.tsx,useDropUpload.ts,useFileVersions.ts,sdk/client.tsPlan 28-03: Eliminate
as anyCastsReplaced 4 polyfill
as anycasts with properdeclare globaltype augmentations. Typed DEV-only window properties via interface extensions.Key files:
polyfills.ts,main.tsx,folder.store.tsPlan 28-04: Archive Legacy POC
Removed
00-Preliminary-R&D/poc/(8 files, 2527 lines, 89 MB committed node_modules). CreatedARCHIVED.mdwith provenance.Key files:
00-Preliminary-R&D/poc/(deleted),00-Preliminary-R&D/ARCHIVED.md(new)Verification
console.*calls in production web code (4 only inside logger.ts itself).catch(() => {})patterns on unpin callsas anycasts in production web code00-Preliminary-R&D/poc/removedKey Decisions
transportshook array for Phase 30 (Grafana Faro) integrationredact()interceptor stripping sensitive fields (privateKey, folderKey, etc.)console.logmapped tologger.debug(suppressed in production)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Chores