Repository navigation
Finish desktop startup resilience on finalized hardening stack - #206
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Deploying network-map with
|
| Latest commit: |
4cc6d53
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://194d0e9c.network-map-dew.pages.dev |
| Branch Preview URL: | https://hardening-startup-resilience.network-map-dew.pages.dev |
|
Warning Review limit reached
Next review available in: 57 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe application now tracks boot diagnostics, captures startup and rendering failures, renders recovery controls, defers optional runtimes, updates accessibility state, and validates these behaviors through a smoke test in CI. ChangesStartup resilience
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@occu-med-map/src/main.tsx`:
- Around line 140-147: Update the missing-root branch in the bootstrap flow to
create a temporary recovery host, render ApplicationFailureScreen into it, and
return without throwing. Preserve recordBootFailure("application-root", ...) and
ensure the recovery UI is displayed when `#root` is absent rather than relying on
boot().catch().
In `@occu-med-map/src/startupDiagnostics.ts`:
- Line 48: Bound the failure history maintained by failures to a fixed maximum
number of recent BootFailure entries, updating the global failure-recording
logic around lines 98-101 to evict older entries when the limit is reached.
Preserve recent failures for diagnostics and track dropped entries if the
existing snapshot or recovery-screen model supports it.
- Around line 141-149: Update markApplicationInteractive and
markOptionalRuntimesComplete to preserve the existing "failed" boot phase: only
transition to "degraded", "interactive", or "ready" when phase is not already
"failed". Ensure scheduled calls cannot overwrite the terminal state set by
recordBootFailure.
- Around line 57-64: Update errorMessage so its JSON.stringify branch always
produces a string, including when serialization returns undefined for values
such as undefined, functions, or symbols. Preserve the existing Error, string,
and serialization-error handling while ensuring recordBootFailure() never
receives an undefined or empty error message.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b5e97632-da0b-4f91-adf2-ddf855bf5355
📒 Files selected for processing (7)
.github/workflows/validate.ymloccu-med-map/package.jsonoccu-med-map/scripts/startup-hardening-smoke.tsoccu-med-map/src/AppErrorBoundary.tsxoccu-med-map/src/main.tsxoccu-med-map/src/startup-hardening.cssoccu-med-map/src/startupDiagnostics.ts
| const rootHost = document.getElementById("root"); | ||
| if (!rootHost) { | ||
| recordBootFailure("application-root", new Error("Network Map root element is missing"), true); | ||
| throw new Error("Network Map root element is missing"); | ||
| } | ||
| const rootElement: HTMLElement = rootHost; | ||
| rootElement.setAttribute("aria-busy", "true"); | ||
| const root = createRoot(rootElement); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Render recovery UI when #root is absent.
This branch records the failure and throws before boot().catch() at Line 193. The application therefore leaves a blank page for the exact root-validation failure that this startup hardening adds.
Create a temporary recovery host in the document and render ApplicationFailureScreen into it instead of throwing.
🤖 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 `@occu-med-map/src/main.tsx` around lines 140 - 147, Update the missing-root
branch in the bootstrap flow to create a temporary recovery host, render
ApplicationFailureScreen into it, and return without throwing. Preserve
recordBootFailure("application-root", ...) and ensure the recovery UI is
displayed when `#root` is absent rather than relying on boot().catch().
|
|
||
| const bootStartedAt = performance.now(); | ||
| const runtimeRecords = new Map<string, RuntimeRecord>(); | ||
| const failures: BootFailure[] = []; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the failure history.
failures grows for the lifetime of the application. Global handlers can add unlimited distinct errors. This contradicts the bounded diagnostics requirement and can make snapshots and the recovery screen increasingly expensive.
Keep a fixed number of recent failures. Record a dropped-entry count if needed.
Also applies to: 98-101
🤖 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 `@occu-med-map/src/startupDiagnostics.ts` at line 48, Bound the failure history
maintained by failures to a fixed maximum number of recent BootFailure entries,
updating the global failure-recording logic around lines 98-101 to evict older
entries when the limit is reached. Preserve recent failures for diagnostics and
track dropped entries if the existing snapshot or recovery-screen model supports
it.
| function errorMessage(error: unknown): string { | ||
| if (error instanceof Error) return error.message || error.name; | ||
| if (typeof error === "string") return error; | ||
| try { | ||
| return JSON.stringify(error); | ||
| } catch { | ||
| return "Unknown startup error"; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -HI -t f -g 'tsconfig*.json' . -x sh -c '
echo "--- $1"
rg -n "\"strict\"|\"strictNullChecks\"" "$1" || true
' sh {}Repository: Occumed79/Network-Map
Length of output: 3798
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- locate file and relevant lines ---"
wc -l occu-med-map/src/startupDiagnostics.ts
sed -n '1,140p' occu-med-map/src/startupDiagnostics.ts
echo "--- tsconfig relevant includes --"
cat -n occu-med-map/tsconfig.json
echo "--- behavioral probes for JSON.stringify return values ---"
node - <<'JS'
for (const value of [undefined, function F(){}, Symbol("s"), null, {}, [], ""]) {
const str = JSON.stringify(value);
console.log(JSON.stringify({value: String(value), returnedValue: str, returnedValueType: str === undefined ? "undefined" : typeof str}));
}
JSRepository: Occumed79/Network-Map
Length of output: 4937
Always return a string from errorMessage.
JSON.stringify(undefined), functions, and symbols return undefined, so recordBootFailure() can record a failure with an empty error and no message.
Proposed fix
try {
- return JSON.stringify(error);
+ return JSON.stringify(error) ?? "Unknown startup error";
} catch {
return "Unknown startup error";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function errorMessage(error: unknown): string { | |
| if (error instanceof Error) return error.message || error.name; | |
| if (typeof error === "string") return error; | |
| try { | |
| return JSON.stringify(error); | |
| } catch { | |
| return "Unknown startup error"; | |
| } | |
| function errorMessage(error: unknown): string { | |
| if (error instanceof Error) return error.message || error.name; | |
| if (typeof error === "string") return error; | |
| try { | |
| return JSON.stringify(error) ?? "Unknown startup error"; | |
| } catch { | |
| return "Unknown startup error"; | |
| } |
🤖 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 `@occu-med-map/src/startupDiagnostics.ts` around lines 57 - 64, Update
errorMessage so its JSON.stringify branch always produces a string, including
when serialization returns undefined for values such as undefined, functions, or
symbols. Preserve the existing Error, string, and serialization-error handling
while ensuring recordBootFailure() never receives an undefined or empty error
message.
| export function markApplicationInteractive(rootElement: HTMLElement): void { | ||
| rootElement.setAttribute("aria-busy", "false"); | ||
| if (readyAt === null) readyAt = elapsed(); | ||
| setBootPhase(failures.length ? "degraded" : "interactive"); | ||
| } | ||
|
|
||
| export function markOptionalRuntimesComplete(): void { | ||
| setBootPhase(failures.length ? "degraded" : "ready"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the terminal boot phase.
A fatal render error sets phase to "failed" in recordBootFailure. The scheduled calls to markApplicationInteractive and markOptionalRuntimesComplete then replace it with "degraded". The recovery UI remains visible, but __NETWORK_MAP_BOOT__.snapshot().phase reports the wrong terminal state.
Keep "failed" unchanged in both functions.
🤖 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 `@occu-med-map/src/startupDiagnostics.ts` around lines 141 - 149, Update
markApplicationInteractive and markOptionalRuntimesComplete to preserve the
existing "failed" boot phase: only transition to "degraded", "interactive", or
"ready" when phase is not already "failed". Ensure scheduled calls cannot
overwrite the terminal state set by recordBootFailure.
1 participant
Follow-up to #205 and surgical replacement for the divergent #167 branch.
This ports only the startup protections that still fit the finalized runtime-ownership architecture. It deliberately does not bring back #167's old mutation-controller changes.
Scope
window.__NETWORK_MAP_BOOT__, including phase, optional-runtime timings, deduplicated failures, and global error/unhandled-rejection capture.aria-busystate and first-interactive-frame tracking.dualMapTransitionRuntimeremains excluded from startup/idle loading.test:startup-hardeningstep in Validate.Preserved #205 invariants
runtimeControllerRegistry.tsremains the application-level observation authority.Target
Desktop application only. Mobile/tablet behavior is not a release gate.
Summary by CodeRabbit
New Features
Bug Fixes
Tests